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

Inconsistent HTTP errors for expired auth token? #506

Closed
isaacd8k opened this issue Jan 16, 2023 · 2 comments
Closed

Inconsistent HTTP errors for expired auth token? #506

isaacd8k opened this issue Jan 16, 2023 · 2 comments
Labels
bug Something isn't working @freshbooks/api

Comments

@isaacd8k
Copy link

isaacd8k commented Jan 16, 2023

When making API requests using the FB client and an expired auth token, different REST endpoints return different error messages. For example:

List Projects

When calling client.projects.list(), we get a nicely formatted http 401 error as described in the docs like so:

{
name: "List Projects",
message: "UNAUTHORIZED",
code: "401"
}

Get Identity

In the exact same scenario, when calling client.users.me(), we receive an error as follows:

{
name: "Get Identity",
message: "This action requires authentication to continue",
code: "unauthenticated"
}

Is this a different error and by design? I notice "unauthenticated" rather than "unauthorized".

We were expecting a response as in the first example, i.e. an HTTP status code in the code field, but if this is by design perhaps we can add the "unauthenticated" code check to our HTTP handlers.

See MRE here

@amcintosh
Copy link
Collaborator

Hi.
The SDK error handling is not particularly consistent across different domains (auth, accounting, projects, etc.).
We are planning to address this in the next little bit, but for now the differing error responses is expected.

@amcintosh amcintosh added bug Something isn't working @freshbooks/api labels Jan 16, 2023
@isaacd8k
Copy link
Author

Ok thanks for the heads up 👍🏼

amcintosh added a commit that referenced this issue Feb 16, 2023
Error responses are inconsistent across resource types (eg. /accounting,
/projects, etc.) and all the handling was done in a single method that
caused inconsistencies (eg. code 401 vs unauthorized, wrong messages,
etc.).

This refactors the error handling so each resource type has its own
method, allowing us to correctly populate the error objects. This
also renames the fields `code` and `number` to `statusCode` and
`errorCode` for clarity.

This helps address [issue 506](#506)),
though it does not solve that completely as the various resources are
still inconsistent in their messaging, but at least the SDK doesn't make
the problem worse.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working @freshbooks/api
Projects
None yet
Development

No branches or pull requests

2 participants