Skip to main content

Bug Tracker

Side navigation

#14923 closed feature (notabug)

Opened March 26, 2014 09:22PM UTC

Closed April 14, 2014 03:29AM UTC

Expose responses to .ajax statusCode handlers

Reported by: Marc_Brooks Owned by:
Priority: undecided Milestone: None
Component: unfiled Version: 2.1.0
Keywords: Cc:
Blocked by: Blocking:
Description

Since a lot of APIs have useful information in the response text (''sometimes plain-text, sometimes HTML, sometimes JSON, etc''), it would be helpful to allow error handlers to see the responses that jQuery went to all the trouble of parsing.

Let's just add the already-parsed response object to:

1. The list of arguments in the deferred.resolveWith or deferred.rejectWith calls. This would modify the call signature of those methods adding a signal argument but that should not break any existing implementations.

2. Set a response property on the jqHR. I'm concerned with this having leak/lifetime implications, but the advantage would be to also have that information available on the jqHR.statusCode handlers.

3. Only pass the response object to the jqHR.statusCode handlers. This would change their call signature from void to a single argument, but since there are very users of this method, and they should only be expecting this pointing at the underlying jqHR, we should be safe. Sadly this would be pretty difficult to hook up properly without doing option 2.

Attachments (0)
Change History (9)

Changed March 26, 2014 10:12PM UTC by Marc_Brooks comment:1

Not sure where I can get a fiddle that makes a 4xx error that includes a response, but here's a starting fiddle http://jsfiddle.net/IDisposable/2LxPW/1/

Changed March 26, 2014 10:24PM UTC by Marc_Brooks comment:2

_comment0: Added a pull request implementing option #1 https://github.com/jquery/jquery/pull/1552 \ \ Kind of shows that it would be nicer to do option #2, but then the response object isn't known to to the success case since we don't pass the jqXHR object to that resolve.1395872759859426

Added a pull request implementing option 1 https://github.com/jquery/jquery/pull/1552

Kind of shows that it would be nicer to do option 2, but then the response object isn't known to to the success case since we don't pass the jqXHR object to that resolve.

Changed March 27, 2014 12:02AM UTC by jaubourg comment:3

The jqXHR object already has response fields (response, responseXML and responseJSON) that are available in both resolved and rejected states. So I'm not sure what you're asking for here.

Changed March 27, 2014 01:21AM UTC by Marc_Brooks comment:4

I don't see that in http://jsfiddle.net/IDisposable/2LxPW/4/ (hitting a twitter endpoint that has a text response to a 401)

The console.log(jqXHR) gives

Object {readyState: 0, getResponseHeader: function, getAllResponseHeaders: function, setRequestHeader: function, overrideMimeType: function…}
abort: function ( statusText ) {
always: function () {
complete: function () {
done: function () {
error: function () {
fail: function () {
getAllResponseHeaders: function () {
getResponseHeader: function ( key ) {
overrideMimeType: function ( type ) {
pipe: function ( /* fnDone, fnFail, fnProgress */ ) {
progress: function () {
promise: function ( obj ) {
readyState: 0
setRequestHeader: function ( name, value ) {
state: function () {
status: 0
statusCode: function ( map ) {
statusText: "error"
success: function () {
then: function ( /* fnDone, fnFail, fnProgress */ ) {
__proto__: Object

What I'm trying to accomplish is when there is an error state, having access to the actual response (hopefully fully parsed for JSON or XML) since we have that already done in the done method of $.ajax.

Changed March 27, 2014 04:05AM UTC by jaubourg comment:5

That seems very fishy to me. responseXXX fields are set in ajaxConvert ( https://github.com/jquery/jquery/blob/master/src/ajax.js#L216 ) which is called for both success and errors ( https://github.com/jquery/jquery/blob/master/src/ajax.js#L691 ). It uses the global option set in ajaxSettings ( https://github.com/jquery/jquery/blob/master/src/ajax.js#L332 ) which references text, XML and JSON dataTypes.

Unless you're using an older version of jQuery or response headers are not set up properly, those fields will be set.

Changed March 27, 2014 04:11AM UTC by jaubourg comment:6

Also, we're supposedly dealing with a 401 but look at the status on the jqXHR instance:

status: 0

Seems to me there's something wrong with the test case.

Changed March 27, 2014 05:40AM UTC by Marc_Brooks comment:7

Ah, but the s (created at https://github.com/jquery/jquery/blob/master/src/ajax.js#L691 ) is NOT the jqXHR (created at https://github.com/jquery/jquery/blob/master/src/ajax.js#L428 ) of line 216 is NOT the jqXHR of line 741 et al. They are different objects entirely. The result of ajaxConvert from https://github.com/jquery/jquery/blob/master/src/ajax.js#L691 is assigned to a local that I just want passed to the deferred resolves of various ilk so I can reach up and get the raw text or JSON or XML as parsed.

You can see from the jsFiddle that it was running against 2.1, but here's a new and improved one against 2.x http://jsfiddle.net/IDisposable/2LxPW/5/ which still shows that the jqXHR does NOT have the responses handy.

Changed March 27, 2014 06:20AM UTC by jaubourg comment:8

You have no response because the actual request was not issued. It's a CORS issue you have here, hence why the status is 0. That's pretty obvious when you open your favorite development tool:

XMLHttpRequest cannot load https://api.twitter.com/oauth/access_token. No 'Access-Control-Allow-Origin' header is present on the requested resource. Origin 'http://fiddle.jshell.net' is therefore not allowed access. 

Have you tried with a same-domain setup? Because we do have unit tests regarding those responseXXX fields: https://github.com/jquery/jquery/blob/master/test/unit/ajax.js#L1391

Also, jqXHR actually is the same object throughout the whole ajax request. Objects are passed by reference in JavaScript.

Changed April 14, 2014 03:29AM UTC by dmethvin comment:9

resolution: → notabug
status: newclosed