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 handling of Rite Aid API errors #537

Closed
Mr0grog opened this issue Jan 18, 2022 · 2 comments · Fixed by #547
Closed

Improve handling of Rite Aid API errors #537

Mr0grog opened this issue Jan 18, 2022 · 2 comments · Fixed by #547
Labels
bug Something isn't working loader

Comments

@Mr0grog
Copy link
Collaborator

Mr0grog commented Jan 18, 2022

Today we logged an API error from Rite Aid, but the error message was just “200 OK,” which is not very helpful: https://sentry.io/organizations/usdr/issues/2940100159

Luckily, more info was buried in the details object, which still go surfaced on Sentry:

{
  details: {
    body: {
      Data: null,
      Status: 'ERROR',
      ErrCde: '400',
      ErrMsg: 'Captcha validation failed. Please retry or contact system administrator',
      ErrMsgDtl: null
    }
  }
}

The RiteAidXhrError class should have given us a nicer message, but the message did not get set correctly for some reason.

class RiteAidXhrError extends HttpApiError {
parse(response) {
super.parse(response);
this.message = `${this.details.Status} ${this.details.ErrCde}: ${this.details.ErrMsg}`;
}
}

@Mr0grog Mr0grog added bug Something isn't working loader labels Jan 18, 2022
@Mr0grog
Copy link
Collaborator Author

Mr0grog commented Jan 18, 2022

Looks like the call to super.parse(response) probably threw an error getting details.error.message:

/**
* Parse an HTTP response for error information and set relevant properties
* on the error instance. This should be overridden by subclasses to handle
* different error formats.
*
* It's safe for this to raise an exception (it will be handled and a more
* generic error message will be set).
*
* @param {http.IncomingMessage} response An HTTP response with error info.
*/
parse(response) {
this.details = JSON.parse(response.body);
this.message = this.details.error.message;
}

…So maybe RiteAidXhrError should just call JSON.parse() instead of super? Otherwise the superclass’s parse method should be written to be less failure-prone.

@Mr0grog
Copy link
Collaborator Author

Mr0grog commented Jan 18, 2022

We should probably also be re-using this error class in the Rite Aid API.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working loader
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant