Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

fix($httpBackend): complete the request on timeout #14972

Closed

Conversation

gkalpak
Copy link
Member

@gkalpak gkalpak commented Jul 31, 2016

What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
Bug fix.

What is the current behavior? (You can also link to an open issue here)
$httpBackend fails to detect a request timeout, when .timeout has been specified on the XHR (or any timeout in Safari 9).
See #14969.

What is the new behavior (if this is a feature change)?
$httpBackend detects request timeouts.

Does this PR introduce a breaking change?
No.

Please check if the PR fulfills these requirements

Other information:
When using the timeout attribute and an XHR request times out, browsers trigger the timeout event (and execute the XHR's ontimeout callback). Additionally, Safari 9 handles timed-out requests in the same way, even if no timeout has been explicitly set on the XHR.
In the above cases, $httpBackend would fail to capture the XHR's completing (with an error), so the corresponding $http promise would never get fulfilled.

Fixes #14969.

When using the [timeout attribute](https://xhr.spec.whatwg.org/#the-timeout-attribute) and an XHR
request times out, browsers trigger the `timeout` event (and execute the XHR's `ontimeout`
callback). Additionally, Safari 9 handles timed-out requests in the same way, even if no `timeout`
has been explicitly set on the XHR.
In the above cases, `$httpBackend` would fail to capture the XHR's completing (with an error), so
the corresponding `$http` promise would never get fulfilled.

Fixes angular#14969
@Narretz
Copy link
Contributor

Narretz commented Aug 2, 2016

So all other browser call the onError handler when no onTimeout is specified?

@gkalpak
Copy link
Member Author

gkalpak commented Aug 2, 2016

Almost 😃 When no timeout (not ontimeout) is specified Chrome calls onerror. Safari calls ontimeout even if not timeout is set on the XHR instance.

(I didn't test other browsers, but since nobody has complained thus far, I assume most of them follow the behavior of Chrome.)

@Narretz
Copy link
Contributor

Narretz commented Aug 2, 2016

Ok, I've tested in FF, IE11 and Edge and they all behave like Chrome (i.e. they call ontimeout only when a timeout is set). I was a bit worried about calling event handlers twice, but I don't think that can happen.
I would mention in the commit message that we don't actually use the ontimeout handler when it's set in http. We use $browser.defer for some reason, and then abort the xhr.

Otherwise, LGTM

@gkalpak gkalpak closed this Aug 8, 2016
@gkalpak gkalpak deleted the fix-httpBackend-complete-ontimeout branch August 8, 2016 16:15
gkalpak added a commit that referenced this pull request Aug 8, 2016
When using the [timeout attribute](https://xhr.spec.whatwg.org/#the-timeout-attribute) and an XHR
request times out, browsers trigger the `timeout` event (and execute the XHR's `ontimeout`
callback). Additionally, Safari 9 handles timed-out requests in the same way, even if no `timeout`
has been explicitly set on the XHR.
In the above cases, `$httpBackend` would fail to capture the XHR's completing (with an error), so
the corresponding `$http` promise would never get fulfilled.

Note that using `$http`'s `timeout` configuration option does **not** rely on the XHR's `timeout`
property (or its `ontimeout` callback).

Fixes #14969
Closes #14972
gkalpak added a commit that referenced this pull request Aug 8, 2016
When using the [timeout attribute](https://xhr.spec.whatwg.org/#the-timeout-attribute) and an XHR
request times out, browsers trigger the `timeout` event (and execute the XHR's `ontimeout`
callback). Additionally, Safari 9 handles timed-out requests in the same way, even if no `timeout`
has been explicitly set on the XHR.
In the above cases, `$httpBackend` would fail to capture the XHR's completing (with an error), so
the corresponding `$http` promise would never get fulfilled.

Note that using `$http`'s `timeout` configuration option does **not** rely on the XHR's `timeout`
property (or its `ontimeout` callback).

Fixes #14969
Closes #14972
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

$http promise does not settle when the connection attempt times out on current Safari
3 participants