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

feat($http): add support for timeout promises #2529

Closed
wants to merge 1 commit into from

Conversation

dbinit
Copy link
Contributor

@dbinit dbinit commented Apr 27, 2013

If the timeout argument is a promise, abort the request when it is resolved.
Implemented by adding support to $httpBackend service and $httpBackend mock service.

This is yet another approach to the $http cancel issue (see #2452 and #2523). Obviously there is still a lack of consensus on how to handle it (see kriskowal/q#64), so I propose this as an alternative.

Closes #1159

@IgorMinar
Copy link
Contributor

I like this quite a bit since unlike all the other attempts it doesn't try to introduce an explicit bi-directional communication between deferred and a promise.

Can you add the missing higher level test please? I'll then highlight this PR one angular-dev for api review. I'm quite hopeful about this PR this time around :)

@ghost ghost assigned IgorMinar May 1, 2013
@dbinit
Copy link
Contributor Author

dbinit commented May 1, 2013

Excellent!

I'll get some more tests in there ASAP. Looks like I'll have to add timeouts to the mock version of $httpBackend, but it shouldn't take long.

@dbinit
Copy link
Contributor Author

dbinit commented May 1, 2013

Ok, tests added.

I did my best to follow the style/spirit of existing code with this (while keeping changes minimal). Let me know if there are any issues there.

If the timeout argument is a promise, abort the request when it is resolved.
Implemented by adding support to $httpBackend service and $httpBackend mock
service.

Closes angular#1159
@dbinit
Copy link
Contributor Author

dbinit commented May 6, 2013

Hi @IgorMinar. Any update on this?

@petebacondarwin
Copy link
Contributor

  • Contributor signed CLA now or in the past (if you just signed, leave a comment here with your real name)
  • Feature is focused on a core value of the framework
  • PR is approved by 2+ senior team members and no committer is blocking the change
  • API is compatible with existing Angular apis and relevant standards (if applicable)
  • Intentional breaking change
    • Approved by 2+ senior team members
    • The breaking change is documented (including migration steps)
  • PR contains unit tests
  • PR contains documentation update
  • PR passes all tests on Travis (sanity)
  • PR passes all tests on ci.angularjs.org (cross-browser compatibility)
  • PR is rebased against recent master
  • PR is squashed into one commit per logical change
  • PR's commit messages are descriptive and allows us to autogenerate release notes (required commit message format)
  • All changes requested in review have been implemented

@petebacondarwin
Copy link
Contributor

I'll ping the dev team on this.

@aaronleesmith
Copy link

Waiting for this to go through to build a feature. Any update?

@vojtajina
Copy link
Contributor

LGTM.

@vojtajina
Copy link
Contributor

🚢 it

@IgorMinar
Copy link
Contributor

Landed as 9f4f593!

Thanks so much for being patient with us.

If you don't have an angular t-shirt please fill out this form and we'll send you one: http://goo.gl/075Sj

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

Successfully merging this pull request may close these issues.

Feature: Cancel $http request
5 participants