-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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 SQS error handling. #14717
Fix SQS error handling. #14717
Conversation
if (response.status > 300) { | ||
throw json | ||
if (response.status >= 300) { | ||
const text = await response.text() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will this always be text?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct me if I'm wrong but I thought .text()
just got the plain old body contents without trying to parse it, so I thought it was always safe to use.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, you are right!
…qs-5xx-error-handling
…qs-5xx-error-handling
…qs-5xx-error-handling
Description
We were attempting to parse the body as JSON before we knew it was JSON, resulting in parse failures before the error handling could kick in.