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

fix($http): immediatelly increment $browser's outstandingRequestCount #13869

Conversation

gkalpak
Copy link
Member

@gkalpak gkalpak commented Jan 28, 2016

This allows protractor to more reliably detect when all outstanding requests have been completed.

Fixes #13782
Closes #13862

@gkalpak
Copy link
Member Author

gkalpak commented Jan 28, 2016

This is an alternative approach to #13862.

Differences:

  • Attempts to decrement the counter a little closer to where it was done before (i.e. before the response interceptors) - not that it should matter afaict.
  • Has a few more tests.
  • Doesn't modify the mock $Browser.

@arashkbanan
Copy link

@arashkbanan

@arashkbanan
Copy link

@Fahimeh

This allows protractor to more reliably detect when all outstanding requests have been completed.

Fixes angular#13782
Closes angular#13862
@gkalpak gkalpak force-pushed the fix-http-increment-request-count-immediately branch from 4b7b565 to c9db640 Compare January 28, 2016 13:07
@Narretz
Copy link
Contributor

Narretz commented Jan 28, 2016

I like it.
Why didn't it work before though? What's the problem with $httpBackend? I think that should go into the commit message.

@gkalpak
Copy link
Member Author

gkalpak commented Jan 28, 2016

The problem (as mentioned in #13782 (comment)) was that $httpBackend was not called until after the promise chain (including the request interceptors) had been processed.
At the very least this requested a $digest, but also note that some request interceptors could return promises (that where also resolved async).

I'll update the commit message.

@petebacondarwin petebacondarwin modified the milestones: 1.5.8, 1.5.x Jul 7, 2016
gkalpak added a commit to gkalpak/angular.js that referenced this pull request Jul 15, 2016
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.

This commit also improves the test coverage of the increment/decrement fix.

Closes angular#13869
@gkalpak gkalpak closed this Jul 15, 2016
@gkalpak gkalpak deleted the fix-http-increment-request-count-immediately branch July 15, 2016 16:39
gkalpak added a commit to gkalpak/angular.js that referenced this pull request Jul 15, 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 angular#13869
gkalpak added a commit to gkalpak/angular.js that referenced this pull request Jul 15, 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 angular#13869
gkalpak added a commit to gkalpak/angular.js that referenced this pull request Jul 15, 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 angular#13869
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
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.

5 participants