Ensure callback not called when request aborted

[stuartsan]

When the xhr is aborted programmatically, the callback should not be called, but it still is. This patch prevents that.

Related to #93, #104

[naugtur]

Am I right that if you abort asynchronously you will get the callback again?

[stuartsan]

@naugtur, do you mean, something like this?

test("aborting XHR prevents callback from being called", { timeout: 500 }, function(assert) {
    var req = xhr({ uri: "/mock/200ok" }, function(err, response) {
        assert.fail('this callback should not be called');
    });

    setTimeout(function() {
      req.abort();
      assert.end()
    }, 0)
})

This test still passes. I think this would depend entirely upon how long the server response takes and when you call req.abort(). If you abort the request before it has completed (maybe with an error, maybe not), this approach should lead to callback prevention.

I believe the core issue was that the onreadystatechange handler is called before the onabort handler in the case of an abort, so this patch flips the order of these, so the onabort handler goes first and gets a chance to say "no do not call the callback".

Does this answer your q?

[naugtur]

Yes, that's it. Would you mind adding that as a test too?

[stuartsan]

Yep no problem, added.

[stuartsan]

Hmmm, I'm not sure why that failed in CI, I'm able to see it pass consistently locally. It might have to do with the mock server response time being faster in CI than on my machine. I'll have to look into this some more.

[stuartsan]

I wonder if there is a way to delay the mock server's response by a fixed amount, say 50ms. Otherwise I'm not sure this second test, the async abort, is very helpful, because it might sometimes pass and sometimes fail, depending on the environment.

[stuartsan]

I had not looked at the mock server...so I swapped in the "timeout" endpoint to simulate a more realistic server response time rather than immediate, like the 200 ok endpoint. This works!