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

fix: better error messages for unsuccessful API responses #23

Merged

Conversation

mcombuechen
Copy link
Collaborator

This adds user-friendly error messages for unsuccessful API responses, such as

  • 400 Bad Request
  • 401 Unauthorized
  • 403 Forbidden
  • other non-200 codes

@mcombuechen mcombuechen requested a review from a team as a code owner March 29, 2023 14:49
@mcombuechen mcombuechen force-pushed the fix/LINK-212-unsuccessful-response branch 2 times, most recently from 6c91b54 to 76e5a7b Compare March 29, 2023 14:58
return extension_errors.New(
err,
fmt.Sprintf(
"Your account is not authorized to perform this action. "+
Copy link
Contributor

Choose a reason for hiding this comment

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

we dont take into account the error messages from the hidden endpoint do we?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

As of now, no we don’t. But I’m open to suggestions here. In a previous iteration the status code was part of the message (Your account is not authorized to perform this action. (Status: 4xx)) but that made it look cryptic to me.

Copy link
Contributor

Choose a reason for hiding this comment

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

In that we should consider removing from the error catalog any errors only used by the hidden endpoint. For example
Although, if it did use the error catalog I could see it being useful for the user to get a link to retrieve their API token; my understanding thats not possible yet for the cli.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, the way it works right now is that we return a Golang error, which basically only has a string value which will be displayed to the user. This is not ideal for many reasons, but most of all that you cannot make a distinction between a cryptic message that would help with debugging, and a user-friendly one that would read nicely and be helpful to our end users. Right now we are prioritizing the user-friendliness and disregarding the internal debugging stuff. This is a temporary solution until the error catalog makes it into the CLI, which should hopefully happen soon. We can then revise all of this and introduce error codes etc. as well.

@mcombuechen mcombuechen force-pushed the fix/LINK-212-unsuccessful-response branch from 76e5a7b to 5b695c9 Compare March 30, 2023 07:46
@mcombuechen mcombuechen merged commit 85f16ec into fix/LINK-200-req-error Mar 30, 2023
@mcombuechen mcombuechen deleted the fix/LINK-212-unsuccessful-response branch March 30, 2023 08:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants