Skip to main content

Bug Tracker

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 anonymous comment:1

workaround is to add

statusCode: {

204: function() {

}

}

But backward comp breaks and AFAIK nothing in the release notes.

Changed January 22, 2013 05:28PM UTC by dmethvin 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 anonymous comment:3

Changed January 22, 2013 09:52PM UTC by anonymous comment:4

Changed January 23, 2013 02:27AM UTC by jaubourg 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 anonymous 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 anonymous 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 jaubourg 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 byroot 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 jaubourg comment:10

component: unfiledajax
milestone: None1.9.1
owner: → jaubourg
priority: undecidedblocker
status: newassigned

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 byroot comment:11

resolution: → fixed
status: assignedclosed

Don't try and convert data for 204 No Content responses. Fixes #13292. Fixes #13261.

Changeset: bfc61b879e9e94ef7c6b31919b7f287ffdf180a8

Changed January 24, 2013 01:37AM UTC by byroot comment:12

Don't try and convert data for 204 No Content responses. Fixes #13292. Fixes #13261.

Changeset: eb47553eeac58861bcefac063c0e03e161b4d52c

Changed January 20, 2014 12:56PM UTC by martyn.frank.87@gmail.com 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 dmethvin 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.