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

fix($http): properly increment/decrement $browser.outstandingRequestCount #14921

Closed

Conversation

gkalpak
Copy link
Member

@gkalpak gkalpak commented Jul 15, 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)
Calling $http will not increment $browser.outstandingRequestCount until after all (potentially
asynchronous) request interceptors have been processed and will decrement it before any (potentially
asynchronous) response interceptors have been processed.
This can lead to $browser.notifyWhenNoOutstandingRequests firing prematurely, which can be
problematic in end-to-end tests.
See also #13782.

What is the new behavior (if this is a feature change)?
The increment/decrement operations are synchronized with $http's internal interceptor promise chain.
The outstandingRequestCount will be incremented as soon as $http() is called and will be decremented after all response interceptors have been processed/fulfilled.

Does this PR introduce a breaking change?
No (I don't think so).

Please check if the PR fulfills these requirements

Other information:
Fixes #13782.
Incorporates #13862 and #13869.

This PR is split up in several commits, so that it is easier to review and easier to selectively "lgtm".
It will be much easier to review commit-by-commit 😄

It includes the following (in order of appearance):

wbyoko and others added 4 commits July 15, 2016 21:24
…Count`

Calling `$http` will not increment `$browser.outstandingRequestCount` until after all (potentially)
asynchronous request interceptors have been processed and will decrement it before any (potentially)
asynchronous response interceptors have been processed.
This can lead to `$browser.notifyWhenNoOutstandingRequests` firing prematurely, which can be
problematic in end-to-end tests.

This commit fixes this, by synchronizing the increment/decrement operations with `$http`'s internal
interceptor promise chain.

Fixes angular#13782
Closes angular#13862
…st fails

Calling `promise.finally(...)` creates a new promise. If we don't return this promise, the user
won't be able to attach an error handler to it and thus won't be able to prevent a potential PUR
error.

This commit also improves the test coverage for the increment/decrement `outstandingRequestCount`
fix.

Closes angular#13869
@googlebot
Copy link

We found a Contributor License Agreement for you (the sender of this pull request) and all commit authors, but as best as we can tell these commits were authored by someone else. If that's the case, please add them to this pull request and have them confirm that they're okay with these commits being contributed to Google. If we're mistaken and you did author these commits, just reply here to confirm.

1 similar comment
@googlebot
Copy link

We found a Contributor License Agreement for you (the sender of this pull request) and all commit authors, but as best as we can tell these commits were authored by someone else. If that's the case, please add them to this pull request and have them confirm that they're okay with these commits being contributed to Google. If we're mistaken and you did author these commits, just reply here to confirm.

@wbyoko
Copy link
Contributor

wbyoko commented Jul 15, 2016

I can confirm I'm okay with these commits being contributed to Google.

@petebacondarwin
Copy link
Contributor

This LGTM but I wonder if it does count as a breaking change for some e2e tests?

petebacondarwin pushed a commit that referenced this pull request Jul 18, 2016
petebacondarwin pushed a commit that referenced this pull request Jul 18, 2016
…st fails

Calling `promise.finally(...)` creates a new promise. If we don't return this promise, the user
won't be able to attach an error handler to it and thus won't be able to prevent a potential PUR
error.

This commit also improves the test coverage for the increment/decrement `outstandingRequestCount`
fix.

Closes #13869
Closes #14921
@gkalpak gkalpak deleted the fix-http-increment-request-count branch July 18, 2016 10:09
gkalpak added a commit that referenced this pull request Jul 19, 2016
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 doesn't immediately increment $brower's outstandingRequestCount. Protractor issues result.
4 participants