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

disable Request onTimeout with async = false #2073

Closed
kguelzau opened this issue Sep 15, 2011 · 5 comments
Closed

disable Request onTimeout with async = false #2073

kguelzau opened this issue Sep 15, 2011 · 5 comments

Comments

@kguelzau
Copy link

Timeout handling does not work with "async: false".
onTimeout is triggered always after xhr.send() returns.

Here is a testcase:
http://jsfiddle.net/kguelzau/thxA4/

Should be

if (!this.options.async) this.onStateChange();
**else** if (this.options.timeout) this.timer = this.timeout.delay(this.options.timeout, this);
@ibolmo
Copy link
Member

ibolmo commented Nov 24, 2011

worksforme.

Here's the deal. If you set async to false then the method Request.send is blocked by line 199 because you asked that it should (remember the async false?).

Since send is blocked and eventually fires the events (e.g. onSuccess) and then sets up the delay, but by then send is all done and the request has already finished.

Re-open if you have another problem.

@ibolmo ibolmo closed this as completed Nov 24, 2011
@kguelzau
Copy link
Author

I don't have the permission to reopen.

It's OK that Request.send is blocking with async=false.
But it doesn't make any sense to me to trigger the timeout delay if i already know that the request definitely has already been finished.
So one option is to remove timeout handling completely (what i suggested) or to start the delay right before send.

@ibolmo ibolmo reopened this Nov 24, 2011
@ibolmo
Copy link
Member

ibolmo commented Nov 24, 2011

Ah I see what you mean, but I think the options are mutually exclusive. You either do async: false or you do timeout: 1000.

If you do async: false the XHR transport will rely on server timeout or browser set timeout (internal to the transport, or perhaps a blocking protection which will prompt user to kill the javascript that is blocking) to return execution to the script.

I've reopened this issue as a documentation issue. Do you agree?

@ibolmo
Copy link
Member

ibolmo commented Nov 24, 2011

@arian @cpojer, we could still reduce headaches if we do as @kguelzau mentioned. To add an else if. If you guys agree with my last comment, I don't see a problem of adding that else if on this line.

@cpojer
Copy link
Member

cpojer commented Nov 24, 2011

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