Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Aborting an XHR still results in a timeout callback #104

Closed
imbcmdth opened this issue Dec 1, 2015 · 5 comments
Closed

Aborting an XHR still results in a timeout callback #104

imbcmdth opened this issue Dec 1, 2015 · 5 comments

Comments

@imbcmdth
Copy link

imbcmdth commented Dec 1, 2015

The issue seems to be around here: https://github.com/Raynos/xhr/blob/master/index.js#L184

When XHRs are aborted, the timeout isn't cleared. When the timer function is invoked, it doesn't first check the status of the XHR before executing the callback.

A possible solution is to listen for the abort event and clear the timeout.

@naugtur
Copy link
Owner

naugtur commented Dec 1, 2015

Confirmed, nice catch. Thanks.

I can't promise I'll fix it this week, so let me know if you'd like to PR this with a test ;)

@TehShrike
Copy link
Collaborator

What's the correct behavior? Should the callback be called with a timeout error right away?

I started to write a test and then realized I didn't know what I should be asserting.

Not calling the callback at all sounds like a way to tempt Zalgo.

@TehShrike
Copy link
Collaborator

Based on issue #93, I propose that

  1. the callback should be called immediately when the abort is fired
  2. the error object should be augmented with err.code = 'ECONNRESET'

Does that sound reasonable? I don't like the idea of no longer calling the callback at all, if only for the reason that it would be a breaking API change.

@naugtur
Copy link
Owner

naugtur commented Jun 4, 2016

We want to mimic how request/request is reacting to .abort()
I checked quickly and it does not call the callback. Nothing else happens after abort.

This issue is about the internal timeout we use instead of the one on XMLHttpRequest object, which doesn't work in some browsers.

Here's a solution https://github.com/Raynos/xhr/tree/abort-timeout, but it'd require extensive cross-browser testing before merging in.

@naugtur naugtur added the bug label Jul 8, 2016
@naugtur naugtur added this to the v2.2.0 milestone Jul 8, 2016
@naugtur
Copy link
Owner

naugtur commented Nov 12, 2016

Looking for volunteers to test in IEs, Safari etc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants