Request module wrongly manage response status code #23

Closed
benouat opened this Issue Jul 20, 2014 · 4 comments

Comments

Projects
None yet
3 participants
Owner

benouat commented Jul 20, 2014

The request module embedded in noder is a bit too restrictive with status code detection when the response get back from the server.

// line 35, in `browser_modules/request.js`
var error = (xhr.status != 200);

I think we should definitely change that code as it is not true at all.
We should do it the other way around, and really list only the status that are considered as errors.

I was playing with a PUT call to the server to create an object in DB, that would usually respond with a status code 201 CREATED. It then goes into the error which is totally wrong

benouat added the bug label Jul 20, 2014

Collaborator

jakub-g commented Jul 21, 2014

I think the most sensible solution would be to treat any 20x status as a success, to be forward-compatible. I.e. check if status is >=200 and < 300 or sth along those lines.

Owner

benouat commented Jul 21, 2014

I had more or less something like that in mind.

Owner

benouat commented Jul 21, 2014

But wait....Error code 30x are not supposed to be considered as error too, aren't they ?

Collaborator

jakub-g commented Jul 21, 2014

It seems that 30x redirects are automatically followed by the browser: http://stackoverflow.com/q/282429/

Not sure if 304 can be hit via XHR, jquery includes this option so probably it is or was:

https://github.com/jquery/jquery/blob/master/src/ajax.js#L680

divdavem self-assigned this Jan 26, 2015

@divdavem divdavem added a commit to divdavem/noder-js that referenced this issue Jan 26, 2015

@divdavem divdavem fix #23 Considers 2xx and 304 http status codes as a success
Considers 2xx and 304 http status codes as a success instead of only 200,
everything else is considered as an error.
ba552f7

@divdavem divdavem added a commit to divdavem/noder-js that referenced this issue Jan 26, 2015

@divdavem divdavem fix #23 Considers 2xx and 304 http status codes as a success
Considers 2xx and 304 http status codes as a success instead of only 200,
everything else is considered as an error.
63aa09d

divdavem closed this in #28 Jan 26, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment