Skip to content
This repository has been archived by the owner on Apr 11, 2024. It is now read-only.

Fix transient json parse error #258

Closed
wants to merge 4 commits into from
Closed

Fix transient json parse error #258

wants to merge 4 commits into from

Conversation

wszaranski
Copy link

@wszaranski wszaranski commented Nov 10, 2021

For all shopify requests we expect to receive json response. When non
json response is received HttpRequestError was thrown instead of
HttpRetriableError.

FIXES #257

Type of change

  • Patch: Bug (non-breaking change which fixes an issue)
  • Minor: New feature (non-breaking change which adds functionality)
  • Major: Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist

  • I have added a changelog entry, prefixed by the type of change noted above
  • I have added/updated tests for this change
  • I have documented new APIs/updated the documentation for modified APIs (for public APIs)

@wszaranski wszaranski requested a review from a team as a code owner November 10, 2021 11:38
@ghost ghost added cla-needed and removed cla-needed labels Nov 10, 2021
@@ -260,6 +260,8 @@ class HttpClient {
.catch((error) => {
if (error instanceof ShopifyErrors.ShopifyError) {
throw error;
} else if (typeof error === 'object' && error.name === 'FetchError' && error.type === 'invalid-json') {

Choose a reason for hiding this comment

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

I might extract this complicated if statement into a function

isRetriableError(error)

For all shopify requests we expect to receive json response. When non
json response is received `HttpRequestError` was thrown instead of
`HttpRetriableError`.

See: #257
Extracted logic to separate function.
Prettier was added to the project so this code had to be adapted to the
change.

See: https://github.com/wszaranski/shopify-node-api/commit/420b799a64fed48be45eea465bf99984b94a3372
It is possible to get 200 response but with non json response. This
should still be treated as error.
@wszaranski wszaranski requested a review from a team as a code owner March 15, 2022 15:15
@JaKXz JaKXz requested review from paulomarg and removed request for a team May 31, 2022 00:14
@JaKXz
Copy link
Contributor

JaKXz commented May 31, 2022

Hey @wszaranski thanks for the PR, but we've decided to go with the solution here in #398 because we wanted to refactor the #doRequest method anyway and we wanted a "simpler" fix. I'm going to close this for now.

@JaKXz JaKXz closed this May 31, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
3 participants