Skip to content

Commit d31e0a8

Browse files
authored
Merge pull request MithrilJS#1612 from bruce-one/xhr-abort
Avoid inaccurately inferring xhr abort.
2 parents a85f595 + a946ef5 commit d31e0a8

File tree

3 files changed

+89
-9
lines changed

3 files changed

+89
-9
lines changed

request/request.js

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22

33
var buildQueryString = require("../querystring/build")
44

5+
var FILE_PROTOCOL_REGEX = new RegExp('^file://', 'i')
6+
57
module.exports = function($window, Promise) {
68
var callbackCount = 0
79

@@ -53,7 +55,16 @@ module.exports = function($window, Promise) {
5355
if (useBody) args.data = args.serialize(args.data)
5456
else args.url = assemble(args.url, args.data)
5557

56-
var xhr = new $window.XMLHttpRequest()
58+
var xhr = new $window.XMLHttpRequest(),
59+
aborted = false,
60+
_abort = xhr.abort
61+
62+
63+
xhr.abort = function abort() {
64+
aborted = true
65+
_abort.call(xhr)
66+
}
67+
5768
xhr.open(args.method, args.url, typeof args.async === "boolean" ? args.async : true, typeof args.user === "string" ? args.user : undefined, typeof args.password === "string" ? args.password : undefined)
5869

5970
if (args.serialize === JSON.stringify && useBody) {
@@ -71,12 +82,13 @@ module.exports = function($window, Promise) {
7182
if (typeof args.config === "function") xhr = args.config(xhr, args) || xhr
7283

7384
xhr.onreadystatechange = function() {
74-
// Don't throw errors on xhr.abort(). XMLHttpRequests ends up in a state of
75-
// xhr.status == 0 and xhr.readyState == 4 if aborted after open, but before completion.
76-
if (xhr.status && xhr.readyState === 4) {
85+
// Don't throw errors on xhr.abort().
86+
if(aborted) return
87+
88+
if (xhr.readyState === 4) {
7789
try {
7890
var response = (args.extract !== extract) ? args.extract(xhr, args) : args.deserialize(args.extract(xhr, args))
79-
if ((xhr.status >= 200 && xhr.status < 300) || xhr.status === 304) {
91+
if ((xhr.status >= 200 && xhr.status < 300) || xhr.status === 304 || FILE_PROTOCOL_REGEX.test(args.url)) {
8092
resolve(cast(args.type, response))
8193
}
8294
else {

request/tests/test-request.js

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -390,6 +390,56 @@ o.spec("xhr", function() {
390390
o(xhr.getRequestHeader("Accept")).equals("application/json, text/*")
391391
}
392392
})
393+
o("doesn't fail on abort", function(done) {
394+
var s = new Date
395+
mock.$defineRoutes({
396+
"GET /item": function() {
397+
return {status: 200, responseText: JSON.stringify({a: 1})}
398+
}
399+
})
400+
401+
var failed = false
402+
var resolved = false
403+
function handleAbort(xhr) {
404+
var onreadystatechange = xhr.onreadystatechange // probably not set yet
405+
var testonreadystatechange = function() {
406+
onreadystatechange.call(xhr)
407+
setTimeout(function() { // allow promises to (not) resolve first
408+
o(failed).equals(false)
409+
o(resolved).equals(false)
410+
done()
411+
}, 0)
412+
}
413+
Object.defineProperty(xhr, 'onreadystatechange', {
414+
set: function(val) { onreadystatechange = val }
415+
, get: function() { return testonreadystatechange }
416+
})
417+
xhr.abort()
418+
}
419+
xhr({method: "GET", url: "/item", config: handleAbort}).catch(function() {
420+
failed = true
421+
})
422+
.then(function() {
423+
resolved = true
424+
})
425+
})
426+
o("doesn't fail on file:// status 0", function(done) {
427+
var s = new Date
428+
mock.$defineRoutes({
429+
"GET /item": function() {
430+
return {status: 0, responseText: JSON.stringify({a: 1})}
431+
}
432+
})
433+
var failed = false
434+
xhr({method: "GET", url: "file:///item"}).catch(function() {
435+
failed = true
436+
}).then(function(data) {
437+
o(failed).equals(false)
438+
o(data).deepEquals({a: 1})
439+
}).then(function() {
440+
done()
441+
})
442+
})
393443
/*o("data maintains after interpolate", function() {
394444
mock.$defineRoutes({
395445
"PUT /items/:x": function() {
@@ -463,5 +513,15 @@ o.spec("xhr", function() {
463513
})
464514
})
465515
})
516+
o("rejects on cors-like error", function(done) {
517+
mock.$defineRoutes({
518+
"GET /item": function(request) {
519+
return {status: 0}
520+
}
521+
})
522+
xhr({method: "GET", url: "/item"}).catch(function(e) {
523+
o(e instanceof Error).equals(true)
524+
}).then(done)
525+
})
466526
})
467527
})

test-utils/xhrMock.js

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ module.exports = function() {
1515
XMLHttpRequest: function XMLHttpRequest() {
1616
var args = {}
1717
var headers = {}
18+
var aborted = false
1819
this.setRequestHeader = function(header, value) {
1920
headers[header] = value
2021
}
@@ -32,18 +33,25 @@ module.exports = function() {
3233
}
3334
this.send = function(body) {
3435
var self = this
35-
var handler = routes[args.method + " " + args.pathname] || serverErrorHandler.bind(null, args.pathname)
36-
var data = handler({url: args.pathname, query: args.search || {}, body: body || null})
36+
if(!aborted) {
37+
var handler = routes[args.method + " " + args.pathname] || serverErrorHandler.bind(null, args.pathname)
38+
var data = handler({url: args.pathname, query: args.search || {}, body: body || null})
39+
self.status = data.status
40+
self.responseText = data.responseText
41+
} else {
42+
self.status = 0
43+
}
3744
self.readyState = 4
38-
self.status = data.status
39-
self.responseText = data.responseText
4045
if (args.async === true) {
4146
var s = new Date
4247
callAsync(function() {
4348
if (typeof self.onreadystatechange === "function") self.onreadystatechange()
4449
})
4550
}
4651
}
52+
this.abort = function() {
53+
aborted = true
54+
}
4755
},
4856
document: {
4957
createElement: function(tag) {

0 commit comments

Comments
 (0)