Handle json parse errors?

[mattdesl]

When opt.json is truthy, errors on JSON.parse() are swallowed.
https://github.com/Raynos/xhr/blob/bc9674d7d5f5c8870ab23fb736fbacc2b3f73518/index.js#L38-L40

Is there a recommended way of handling these errors while still supporting json: String option?

[TehShrike]

I can understand why you might want to return the body as-is in that case - how even would you report these kinds of errors? Have an event emitter somewhere that would emit parse errors or something? I'm not sure what I would want (but I can agree I would probably want to see something if this happened).

[mattdesl]

I would assume just return Error in first callback, maybe with a code that can be used to identify the issue?

Or: just leave it as is but document it and encourage users to JSON.parse themselves if they want to be safe.

[TehShrike]

It would be really weird to me if it returned an error as the first argument to the callback, but then also included the un-parsed body of the response.

Right now it just returns the un-parsed body of the response.

[naugtur]

IT would actually make sense to do that to give the consumer a chance to recover, but if the server returns invalid JSON with JSON content-type, there's no good way to proceed.

Anybody willing to compare with https://github.com/request/request?

[prawnsalad]

I've just been tripped up by this for a while. If I set json:true then I expect JSON to be returned, not a string. But due to a JSON parsing issue I ended up trying to iterate over the invalid JSON string causing headaches instead.

We should get an error stating that parsing the JSON failed. As it currently stands, we have to manually parse the JSON ourselves or check the type of response.body just to safeguard against it.

[naugtur]

I know. That's a behavior that was there long time ago and it's a total breaking change, so requires a major release.