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

check for error _before_ parsing response as JSON/text/whateves #110

Closed
tomwayson opened this issue Feb 1, 2018 · 2 comments · Fixed by #131
Closed

check for error _before_ parsing response as JSON/text/whateves #110

tomwayson opened this issue Feb 1, 2018 · 2 comments · Fixed by #131
Assignees

Comments

@tomwayson
Copy link
Member

It seems to me that we need a response.ok check before this switch:

switch (params.f) {
case "json":
return response.json();
case "geojson":
return response.json();
case "html":
return response.text();
case "text":
return response.text();
/* istanbul ignore next blob responses are difficult to make cross platform we will just have to trust the isomorphic fetch will do its job */
case "image":
return response.blob();
/* istanbul ignore next blob responses are difficult to make cross platform we will just have to trust the isomorphic fetch will do its job */
case "zip":
return response.blob();
}

The same way ember-arcgis-portal-services does here:

https://github.com/Esri/ember-arcgis-portal-services/blob/master/addon/mixins/service-mixin.js#L90-L108

What happens now is that the underlying HTTP error (404, 500, etc) gets buried by a JSON parsing error like SyntaxError: Unexpected token < in JSON at position 0, at least in the case of f=json.

See the first error on https://esri.github.io/ember-cli-cedar/#/error-handling as an example.

If y'all agree, I can PR the logic from ember-arcgis-portal-services.

@patrickarlt
Copy link
Contributor

Seems reasonable to me.

@patrickarlt
Copy link
Contributor

Since I had to know about why this happens according to https://www.tjvantoll.com/2015/09/13/fetch-and-errors/ and https://developer.mozilla.org/en-US/docs/Web/API/Fetch_API/Using_Fetch#Checking_that_the_fetch_was_successful fetch only rejects the promise when a network error is encountered. So you still have to check response.ok and return a Promise.reject(response) if the response is not ok make this behave like most people will expect.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants