Skip to content

Dont throw/catch transform exceptions on 304#3036

Closed
pebl-work wants to merge 3 commits intoaxios:masterfrom
pebl-work:patch-1
Closed

Dont throw/catch transform exceptions on 304#3036
pebl-work wants to merge 3 commits intoaxios:masterfrom
pebl-work:patch-1

Conversation

@pebl-work
Copy link
Copy Markdown

@pebl-work pebl-work commented Jun 18, 2020

Dont try to parse response data from 304 requests, as it is suppose to not have a body.

The real problem is A)

var request = new XMLHttpRequest();

in some browsers set response/responseText to "" in XMLHttpRequest.

At this point B)

data: responseData,

the response/responseText is copied directly without checking if there should be a body; copying "".

The same problem at this place: C)

response.data,

And finally at the patched line, where the "" is attempted to be parsed as JSON, which it is not, nor should be.

I dont think A) will change, and changing B+C) is a lot of complex code for very little.

Dont try to parse response data from 304 requests, as it is suppose to not have a body.

The real problem is A)
https://github.com/axios/axios/blob/36f0ad2f985c3289018f0fdaaddf309cc9458d9b/lib/adapters/xhr.js#L28
in some browsers set response/responseText to "" in XMLHttpRequest.

At this point B)
https://github.com/axios/axios/blob/36f0ad2f985c3289018f0fdaaddf309cc9458d9b/lib/adapters/xhr.js#L61
the response/responseText is copied directly without checking if there should be a body; copying "".

The same problem at this place: C)
https://github.com/axios/axios/blob/5b08fc4ac7ecc896efa37952645ea578a3609fc2/lib/core/dispatchRequest.js#L56

And finally at the patched line, where the "" is attempted to be parsed as JSON, which it is not, nor should be.

I dont think A) will change, and changing B+C) is a lot of complex code for very little.
@jasonsaayman
Copy link
Copy Markdown
Member

@pebl-work please can you fix the conflicts in this branch, update it to the latest master, and let me know so I can review it fully.

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 this pull request may close these issues.

3 participants