Side navigation
#13292 closed bug (fixed)
Opened January 21, 2013 11:52PM UTC
Closed January 24, 2013 01:37AM UTC
Last modified January 20, 2014 02:36PM UTC
$.ajax with 1.9.0 doesn't call anymore success function in case of 204
Reported by: | olamy@apache.org | Owned by: | jaubourg |
---|---|---|---|
Priority: | blocker | Milestone: | 1.9.1 |
Component: | ajax | Version: | 1.9.0 |
Keywords: | Cc: | jaubourg | |
Blocked by: | Blocking: |
Description
Using $.ajax success function.
In case of 204 (No content) success function is not called anymore
Attachments (0)
Change History (14)
Changed January 22, 2013 07:53AM UTC by comment:1
Changed January 22, 2013 05:28PM UTC by comment:2
cc: | → jaubourg |
---|
@jaubourg can you take a look? Seems like it should be calling the success callback.
Changed January 22, 2013 09:28PM UTC by comment:3
Duplicate of http://bugs.jquery.com/ticket/13261
Changed January 22, 2013 09:52PM UTC by comment:4
Pull request: https://github.com/jquery/jquery/pull/1142
Changed January 23, 2013 02:27AM UTC by comment:5
Could we have a test case that doesn't use mockjax? Because the test case in #13261 doesn't fail if and when you remove the contentType option. It doesn't make any sense since the option is request-related not response-related. So I really think something's fishy here.
The mockjax repo says "The current version of Mockjax has been tested with jQuery 1.3.2 through 1.7.0 with QUnit unit tests, residing in /test." While it's more than likely that it has been tested with earlier versions, it wouldn't hurt to check.
Changed January 23, 2013 03:13PM UTC by comment:6
Replying to [comment:5 jaubourg]:
Could we have a test case that doesn't use mockjax? Because the test case in #13261 doesn't fail if and when you remove the contentType option. It doesn't make any sense since the option is request-related not response-related. So I really think something's fishy here.
Unfortunately jsFiddle can't be used to respond with a 204/No Content.
You might want to have a look at the jQuery function ajaxConvert which takes the request into account when creating the converters. This might be the magic thing happening behind the scenes - I don't believe it's related to mockjax.
Also, the Content-Type header in the request is correct since - in my case - the request sends some JSON, it's just the fact that the response is empty.
Is is possible that jQuery expects a JSON response if JSON has been sent?
Changed January 23, 2013 03:57PM UTC by comment:7
Replying to [comment:4 anonymous]:
Pull request: https://github.com/jquery/jquery/pull/1142
+1
That's exactly what should happen!
Changed January 23, 2013 04:24PM UTC by comment:8
Unfortunately jsFiddle can't be used to respond with a 204/No Content.
Could you then provide a test case somewhere on the web? A URL on a server of yourse maybe.
You might want to have a look at the jQuery function ajaxConvert which takes the request into account when creating the converters. This might be the magic thing happening behind the scenes - I don't believe it's related to mockjax.
Everything is in "I don't believe". Have you reproduced this without mockjax? If yes, could you provide a url demonstrating the issue?
Also, the Content-Type header in the request is correct since - in my case - the request sends some JSON, it's just the fact that the response is empty.
I'm not saying it's not correct. I'm just saying it doesn't make sense in relation to the bug. contentType
simply sets a *request* header: it doesn't change anything regarding the response. That dataType
would have an impact, I would understand, but contentType
? Hence why I suspect a problem with mockjax and why I'd like to have a clean, pure ajax example.
Is is possible that jQuery expects a JSON response if JSON has been sent?
In jQuery? As per the previous paragraph: no, it is not. I dunno if there is some logic like this in mockjax.
>> Pull request: https://github.com/jquery/jquery/pull/1142
>
+1 That's exactly what should happen!
Except we never had a test specific to 204 at that level in the code before so the (eventual) regression is elsewhere. I'd still would like to see a test case demonstrating the issue without mockjax (even if only a unit test in a PR).
Changed January 23, 2013 07:41PM UTC by comment:9
Here a JSFiddle that show the error using Github API: http://jsfiddle.net/arkVR/7/
Note that you have to provide your GIthub username and passwords.
You will see that in the second request it's the error callabck that is called even if the server successfully deleted the gist
And this version using jQuery 1.8.3: http://jsfiddle.net/arkVR/8/ call the success callback.
So yeah it's a regression.
Changed January 24, 2013 01:09AM UTC by comment:10
component: | unfiled → ajax |
---|---|
milestone: | None → 1.9.1 |
owner: | → jaubourg |
priority: | undecided → blocker |
status: | new → assigned |
Merci, Jean. Much appreciated test case.
This won't fail with any other dataType than "json" because the regression is due to the re-alignment of parseJSON with native JSON.parse (throwing an exception for null/undefined values).
So THAT is the regression.
Thing is it showed a fault in the ajax code: it should not assume parsers will bypass on null/undefined inputs. The test for 204 is indeed needed but was never there because our parsers never .
I'm taking ownership. The PR is fine but there's a shorter path (in terms of bytes) and I can add a test case.
Changed January 24, 2013 01:37AM UTC by comment:11
Changed January 24, 2013 01:37AM UTC by comment:12
Changed January 20, 2014 12:56PM UTC by comment:13
I can see that this was released in jQuery 2, but is there any plans to release it in jQuery 1?
Changed January 20, 2014 02:36PM UTC by comment:14
Replying to [comment:13 martyn.frank.87@…]:
I can see that this was released in jQuery 2, but is there any plans to release it in jQuery 1?
The problem shouldn't exist in the 1.x branch. If it does, file a new ticket with a test case.
workaround is to add
statusCode: {
204: function() {
}
}
But backward comp breaks and AFAIK nothing in the release notes.