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

FIX wrong success for an erronous fetch, after an abort of a legit fetch #22

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

basos9
Copy link

@basos9 basos9 commented Dec 13, 2012

This bug can be manifested when a pending fetch of a legit resource (200 response)
is aborted and afterwards a new fetch that leads to error (e.g. 404) is issued.
At the time, the second request finishes the lastValue has the aborted data of
the previous fetch (callback is executed even on aborted), YIELDING the second (normally
erroneus response) to be signaled as successfull.

The effect of this bug is demonstrated with a test,
One can remove the jsonp patch and see what happens.

basos9 added 2 commits December 13, 2012 16:12
Conflicts:
	test/unit.js
This bug can be manifested when a pending fetch of a legit resource (200 response)
is aborted and afterwards a new fetch that leads to error (e.g. 404) is issued.
At the time, the second request finishes the lastValue has the aborted data of
the previous fetch (callback is executed even on aborted), YIELDING the second (normally
erroneus response) to be signaled as successfull.
@jaubourg
Copy link
Owner

Isn't this due to the fact browsers won't stop the execution of a script when the script tag is removed from the DOM?

@basos9
Copy link
Author

basos9 commented Feb 14, 2013

Yes, this seems to be the basic cause. Of course this applies when the callback name is the same across requests. If it is random the odds decrease.
I think that this fix leaves a corner case, but it definitely narrows the error cases.
For example; this will work (with the fix), 1.request, 2. abort, 3. aborted 200 response (modifies lastValue), 4. request, 5. response 404, 6. notify error (since lastValue is reset at 4).
But this still does not work 1. request, 2.abort, 3. request (reset lastValue), 4. aborted 200 response (modifies lastValue), 5. response 404, 6. notify success (since lastValue is considered legit from 4).
But i think that the last case is more rare or maybe impossible. The last will be valid if one thing that I'm not sure if it applies in this case, is valid: script execution is done in the order included and serially (anyway). This applies at least for handwritten script tags. But I think it should apply for dynamic scripts as well.

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

Successfully merging this pull request may close these issues.

2 participants