Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

improve error handling #3

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

angusfretwell
Copy link
Contributor

@angusfretwell angusfretwell commented Sep 21, 2024

Requests will now throw a CiviCRMRequestError with the message provided by the API, if any, and a detail property containing any other details provided by the API.

Copy link

@IanEdington IanEdington left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Happy to provide feedback if you're looking for it otherwise disregard :D

This is loads better than what exists so LGTM.

@@ -102,3 +92,47 @@ export async function request(

return json.values;
}

function handleError(errorResponse: string | object) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could add a return type of never to this function.
https://www.typescriptlang.org/docs/handbook/2/functions.html#never

Comment on lines +114 to +121
return error;
}

try {
const object = JSON.parse(error);

if (object && typeof object === "object") {
return object;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In both these return cases, there's an assumption that the objects contain an error_message. One way you could solve this is to move the || "CiviCRM request failed", from line 126 to line 135.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants