axios: How to handle error from JSON.parse
The transformRequest method attempts to parse JSON, but swallows any error in an empty catch statement. This results in the stringified form of the response body to be returned should the JSON be malformed, and supplied to the response object that is used to resolve the Promise.
Without the try/catch a global error would occur. With the try/catch the code handling the response will error, but in an unpredictable way. Consider:
// Assume a malformed response from the server for the request.
// Missing quotes around property names:
// {firstName: "Jimmy", lastName: "Page", isBlocked: true}
axios.get('/user/12345').then((res) => {
var user = res.data;
if (!user.isBlocked) {
// Do something potentially dangerous that only an unblocked user should be allowed to do
}
});
In this case user is a String not an Object as expected. Calling 'some string'.isBlocked will return undefined, which evaluates as falsey. This will cause the if condition to erroneously be entered.
A potential solution would be to reject the Promise if JSON.parse throws an Error. This isn’t a straight forward fix though as it would break #55 again.
About this issue
- Original URL
- State: closed
- Created 9 years ago
- Reactions: 24
- Comments: 21 (2 by maintainers)
Commits related to this issue
- JSON improvements: throw if JSON parsing failed; number, boolean can be passed directly as payload for encoding to JSON #2613, #61, #907 (#3688) * Draft * Added support for primitive types to be c... — committed to axios/axios by DigitalBrainJS 3 years ago
- JSON improvements: throw if JSON parsing failed; number, boolean can be passed directly as payload for encoding to JSON #2613, #61, #907 (#3688) * Draft * Added support for primitive types to be c... — committed to mbargiel/axios by DigitalBrainJS 3 years ago
Just got burned on this. Instead of correctly rejecting the promise on invalid (truncated) JSON, it simply returns the orignal string object. Bad bad bad. At least put a warning in the docs about this.
@emilyemorehouse is this something you could fix, for the sake of all humanity?
I think we should just reject with the parse errors. That’d make axios more predictable and easier to debug.
Not sure how others are avoiding this issue after almost 3 years, but I decided to try to work around it for one of my projects. I would like to see how other people have been doing this as well.
I took notes from here and only raise an error for 2xx response codes.
If people find it useful I can make a PR of my commit. If you need this to raise for other response codes, just remove lines 67 and 71 in
defaults.jsAnother option for those who don’t want to fork is to just override transformResponse, and detect when data is a string, and parse on your own. Unfortunately this means you’ll do more work, and you can’t check the status code if you do that, but at least you have options.
I also spent 1 hour today tryng to understand what is happening with some invalid JSON. Just reject the Promise, or at least console.log something…?
imagine first finding out in production - not sure where the logical thinking has gone on this one
Personally, I think Promise rejection is a good solution. At the very least, one could invoke
console.errorin thecatchblock to make developers aware of what has happened.I see this has been closed already, but imo neither of the available behaviours from silentJSONParsing are quite perfect for all use cases. With silentJSONParsing on you can access the status code of the response, but the JSON parsing error is swallowed which isn’t ideal. But with silentJSONParsing off there’s no longer any way to see what the status code of the response was, unless I’m missing something.
Effectively it seems like I have to choose between being able to know the status code or the JSON parsing error, but not both.
Fixed in #3688
This is among the most disappointing responses to a glaring issue I’ve seen in an open-source project. This has been an issue for almost 4 years… This is the reason I use my own HTTP package.
I just got bitten by this in production today. The current behavior is really unintuitive and wasted a bunch of my time in debugging. Throwing an error would have pointed me in the right direction immediately and would have been helpfully caught/grouped in our error reporting. Even once I found that there was a disconnect between the type of data I got in the response vs. what the header said it should be, it wasn’t trivial to find this issue as the explanation.
+1 here, swallowing errors is never good
I also think there should be an easy option to retry the request in these cases as it often boils down to a truncated response.