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

Always return code or error in responses as ErrorCode when present #203

Closed
wants to merge 1 commit into from

Conversation

mthadley
Copy link
Contributor

@mthadley mthadley commented Apr 13, 2023

Description

We currently set ErrorCode in HTTPError from the code field in raw responses from the WorkOS API, but only in certain cases, like getting a 422.

Otherwise, ErrorCode ends up as empty string in other cases. For example, as noted in #198, when an invalid Authorization Code is provided to the token endpoint, the following raw error is returned:

{ 
  "error": "invalid_grant", 
  "error_description": "The code '01E2RJ4C05B52KKZ8FSRDAP23J' has expired or is invalid." 
}

However, the ErrorCode is not populated, making it hard to handle this (or other) particular errors.

This PR updates the error parsing logic to always attempt to populate ErrorCode with code from the response, falling back to error when not present. In all cases I'm aware of, error should be a discrete error type identifier suitable for dispatching on in response handling code.

Documentation

Does this require changes to the WorkOS Docs? E.g. the API Reference or code snippets need updates.

[ ] Yes

If yes, link a related docs PR and add a docs maintainer as a reviewer. Their approval is required.

@mthadley mthadley requested a review from a team as a code owner April 13, 2023 19:31
@mthadley mthadley requested review from drewfradette and removed request for a team April 13, 2023 19:31
@mthadley mthadley closed this Jun 3, 2024
@mthadley mthadley deleted the expose-more-errors-as-error-code branch June 3, 2024 18:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants