Opened 5 years ago

Closed 5 years ago

#18088 closed defect (invalid)

Possible bug in dojo/request/xhr

Reported by: Jared Jurkiewicz Owned by: Bryan Forbes
Priority: blocker Milestone: tbd
Component: IO Version: 1.10.0-beta1
Keywords: Cc:
Blocked By: Blocking:

Description

I think there is a logic error in dojo/request/xhr. Specifically, it calls the handlers (json, xml, etc), before it checks the actual status of the xhr. So if you say get a 504, and junk back, handleAs JSON fails, but the error you get is a parse error instead of knowing it parse errored BECAUSE the status was non-success. So, shouldn't this be:

if(util.checkStatus(_xhr.status)){

if(error){

this.reject(error);

}else{

this.resolve(response);

}

}else{

error = new RequestError?('Unable to load ' + response.url + ' status: ' + _xhr.status, response); this.reject(error);

}

Instead?

Change History (5)

comment:1 Changed 5 years ago by Jared Jurkiewicz

Priority: undecidedblocker

comment:2 Changed 5 years ago by bill

Component: CoreIO
Owner: set to Bryan Forbes
Status: newassigned

AFAICT this isn't a regression, so it shouldn't block the 1.10 release.

comment:3 Changed 5 years ago by Jared Jurkiewicz

Pull request to fix error reporting order: https://github.com/dojo/dojo/pull/92

comment:4 Changed 5 years ago by ben hockey

isn't it possible that people rely on the current behavior to parse a json response in the case of something like authorization failing (status 401)? won't this change regress what they have come to rely on?

comment:5 Changed 5 years ago by ben hockey

Resolution: invalid
Status: assignedclosed

as discussed on github, the desired result can be achieved using the response promise

Note: See TracTickets for help on using tickets.