Skip to content

Loading…

Pass abort/timeout statuses and xhr.statusText to global ajaxError event handlers #695

Merged
merged 3 commits into from

3 participants

@gilmoreorless

I noticed that since commit b788839 the "abort" status is passed to the inline error callback (which is good).
Unfortunately this wasn't also passed up to the global ajaxError event (nor were "timeout" statuses).

While fixing this for a project I also passed through the HTTP status text (if available) to the error handlers in the event of a 4xx or 5xx response. This more closely matches the jQuery ajax error handlers.

I've also added a couple of tests that were missing: JSON parse errors and XHR abort.

Cheers,
Gil

PS. As a side note, I've made a test page for comparing the callback signatures between Zepto and jQuery: http://zepto-gildev.herokuapp.com/test/compare_ajax.html
A red row just indicates a difference in signatures, not necessarily a problem (in some cases Zepto gives more information).

Would it be valuable to fix up some of the other differences? I think Zepto not firing error callbacks for a beforeSend cancellation is a bug

@mislav mislav was assigned
@madrobby
Owner

Hey, this all looks pretty good—do you have a little bit of time to update this based on the latest master?

@gilmoreorless

Sure thing, I've merged in the latest master and updated the files.

@madrobby madrobby merged commit 351226f into madrobby:master

1 check passed

Details default The Travis CI build passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
Showing with 63 additions and 6 deletions.
  1. +2 −2 src/ajax.js
  2. +54 −1 test/ajax.html
  3. +7 −3 test/server.coffee
View
4 src/ajax.js
@@ -55,7 +55,7 @@
function ajaxError(error, type, xhr, settings) {
var context = settings.context
settings.error.call(context, xhr, type, error)
- triggerGlobal(settings, context, 'ajaxError', [xhr, settings, error])
+ triggerGlobal(settings, context, 'ajaxError', [xhr, settings, error || type])
ajaxComplete(type, xhr, settings)
}
// status: "success", "notmodified", "error", "timeout", "abort", "parsererror"
@@ -225,7 +225,7 @@
if (error) ajaxError(error, 'parsererror', xhr, settings)
else ajaxSuccess(result, xhr, settings)
} else {
- ajaxError(null, xhr.status ? 'error' : 'abort', xhr, settings)
+ ajaxError(xhr.statusText || null, xhr.status ? 'error' : 'abort', xhr, settings)
}
}
}
View
55 test/ajax.html
@@ -293,6 +293,26 @@
)
},
+ testAjaxJSONParserError: function(t){
+ t.pause()
+ $.ajax({
+ url: 'json',
+ dataType: 'json',
+ data: { invalid: true },
+ success: t.reg.handler('success'),
+ error: t.reg.handler('error'),
+ complete: t.reg.resumeHandler('complete', function(){
+ t.assertEqualList('ajaxBeforeSend ajaxSend error ajaxError complete ajaxComplete', t.reg.events())
+ var errorArgs = t.reg.args('error')
+ t.assertEqual('parsererror', errorArgs[1])
+ t.assert(errorArgs[2] instanceof SyntaxError)
+ var globalErrorArgs = t.reg.args('ajaxError')
+ t.assert(globalErrorArgs[3] instanceof SyntaxError)
+ t.assertEqual(0, $('script[src^=slow]').size())
+ })
+ })
+ },
+
testAjaxGetJSONP: function(t){
t.pause()
$.getJSON(
@@ -387,6 +407,8 @@
t.assertEqualList('ajaxBeforeSend ajaxSend error ajaxError complete ajaxComplete', t.reg.events())
var errorArgs = t.reg.args('error')
t.assertEqual('timeout', errorArgs[1])
+ var globalErrorArgs = t.reg.args('ajaxError')
+ t.assertEqual('timeout', globalErrorArgs[3])
t.assertEqual(0, $('script[src^=slow]').size())
})
})
@@ -409,6 +431,8 @@
t.assertEqualList('ajaxBeforeSend ajaxSend error ajaxError complete ajaxComplete', t.reg.events())
var errorArgs = t.reg.args('error')
t.assertEqual('abort', errorArgs[1])
+ var globalErrorArgs = t.reg.args('ajaxError')
+ t.assertEqual('abort', globalErrorArgs[3])
t.assertEqual(0, $('script[src^=slow]').size())
})
}, 50)
@@ -1022,8 +1046,12 @@
},
testTimeout: function(t) {
- var successFired = false, xhr, status
+ var successFired = false, xhr, status, globalStatus
t.pause()
+ $(document).on('ajaxError', function (e, x, o, s) {
+ globalStatus = s
+ })
+
$.ajax({
timeout: 30,
success: function() { successFired = true },
@@ -1040,10 +1068,35 @@
t.assertFalse(successFired)
t.assertTrue(xhr.aborted)
t.assertEqual('timeout', status)
+ t.assertEqual('timeout', globalStatus)
})
}, 40)
},
+ testAbort: function(t) {
+ var successFired = false, xhr, status, globalStatus
+ t.pause()
+ $(document).on('ajaxError', function (e, x, o, s) {
+ globalStatus = s
+ })
+
+ $.ajax({
+ success: function() { successFired = true },
+ error: function(x, s) { xhr = x, status = s }
+ })
+ MockXHR.last.abort()
+ MockXHR.last.ready(4, 0, 'Aborted')
+
+ setTimeout(function(){
+ t.resume(function(){
+ t.assertFalse(successFired)
+ t.assertTrue(xhr.aborted)
+ t.assertEqual('abort', status)
+ t.assertEqual('abort', globalStatus)
+ })
+ }, 20)
+ },
+
testAsyncDefaultsToTrue: function(t) {
$.ajax({ url: '/foo' })
t.assertTrue(MockXHR.last.async)
View
10 test/server.coffee
@@ -39,9 +39,13 @@ app.get '/test/jsonpBlah', (req, res) ->
app.get '/test/json', (req, res) ->
if /json/.test req.headers['accept']
- res.json
- query: req.query
- hello: 'world'
+ if req.query.invalid
+ res.set 'Content-Type', 'application/json'
+ res.send 'invalidJSON'
+ else
+ res.json
+ query: req.query
+ hello: 'world'
else
res.send 400, 'FAIL'
Something went wrong with that request. Please try again.