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

fix($resource): remove (broken) support for _promise as timeout_ #13393

Closed
3 tasks done
gkalpak opened this issue Nov 27, 2015 · 1 comment
Closed
3 tasks done

fix($resource): remove (broken) support for _promise as timeout_ #13393

gkalpak opened this issue Nov 27, 2015 · 1 comment
Assignees
@gkalpak
Labels
component: ngResource frequency: low severity: broken expected use type: bug type: chore type: docs
Milestone
1.4.9

Comments

@gkalpak
Copy link
Member

gkalpak commented Nov 27, 2015

7170f9d which added support for using a promise as timeout in $resource, introduced a bug (where you had to re-create the resource class after every cancelled request and failing to do so would abort all subsequent calls). See #12657 (comment) for more context.

This has been fixed in v1.5.x with #13210, but we need to also fix it on v1.4.x (#13210 is not suitable for backporting to v1.4.x since it adds a new feature and changes the behaviour of $resource to some extent).

As discussed in #13210 (comment), for v1.4.x we need to:

  • Revert 7170f9d.
  • Update the docs to explain that promise as timeout is not supported.
  • Log a warning when someone does it.
@gkalpak gkalpak added this to the 1.4.9 milestone Nov 27, 2015
@gkalpak gkalpak self-assigned this Nov 27, 2015
gkalpak added a commit to gkalpak/angular.js that referenced this issue Dec 7, 2015
@gkalpak
revert: fix($resource): allow XHR request to be cancelled via timeout…
77d4a32 
… promise

This reverts commit 7170f9d.

Fixes part of angular#13393.
gkalpak added a commit to gkalpak/angular.js that referenced this issue Dec 7, 2015
@gkalpak
docs($resource): add note about _promises as timeout_ not being suppo…
96a6b7c 
…rted

Fixes part of angular#13393.
gkalpak added a commit to gkalpak/angular.js that referenced this issue Dec 7, 2015
@gkalpak
fix($resource): don't allow using promises as timeout and log a war…
ed915fe 
…ning

Promises never worked correctly as values for `timeout` in `$resource`, because the same value has
to be re-used for multiple requests (and it is not possible to `angular.copy()` a promise).
Now (in addition to ignoring a non-numeric `timeout`), a warning is logged to the console using
`$log.debug()`.

Partly fixes angular#13393.
@gkalpak gkalpak mentioned this issue Dec 7, 2015
Closed
gkalpak added a commit to gkalpak/angular.js that referenced this issue Dec 8, 2015
@gkalpak
docs($resource): add note about _promises as timeout_ not being suppo…
96e7d80 
…rted

Fixes part of angular#13393.
gkalpak added a commit to gkalpak/angular.js that referenced this issue Dec 8, 2015
@gkalpak
fix($resource): don't allow using promises as timeout and log a war…
66d1330 
…ning

Promises never worked correctly as values for `timeout` in `$resource`, because the same value has
to be re-used for multiple requests (and it is not possible to `angular.copy()` a promise).
Now (in addition to ignoring a non-numeric `timeout`), a warning is logged to the console using
`$log.debug()`.

Partly fixes angular#13393.
gkalpak added a commit to gkalpak/angular.js that referenced this issue Dec 8, 2015
@gkalpak
fix($resource): don't allow using promises as timeout and log a war…
7fe6201 
…ning

Promises never worked correctly as values for `timeout` in `$resource`, because the same value has
to be re-used for multiple requests (and it is not possible to `angular.copy()` a promise).
Now (in addition to ignoring a non-numeric `timeout`), a warning is logged to the console using
`$log.debug()`.

Partly fixes angular#13393.
gkalpak added a commit to gkalpak/angular.js that referenced this issue Dec 8, 2015
@gkalpak
docs($resource): add note about _promises as timeout_ not being suppo…
0c587ea 
…rted

Fixes part of angular#13393.
gkalpak added a commit to gkalpak/angular.js that referenced this issue Dec 8, 2015
@gkalpak
fix($resource): don't allow using promises as timeout and log a war…
6d5141e 
…ning

Promises never worked correctly as values for `timeout` in `$resource`, because the same value has
to be re-used for multiple requests (and it is not possible to `angular.copy()` a promise).
Now (in addition to ignoring a non-numeric `timeout`), a warning is logged to the console using
`$log.debug()`.

Partly fixes angular#13393.
gkalpak added a commit that referenced this issue Dec 8, 2015
@gkalpak @petebacondarwin
revert: fix($resource): allow XHR request to be cancelled via timeout…
ff228fb 
… promise

This reverts commit 7170f9d.

Fixes part of #13393.
gkalpak added a commit that referenced this issue Dec 8, 2015
@gkalpak @petebacondarwin
docs($resource): add note about _promises as timeout_ not being suppo…
0292e6a 
…rted

Fixes part of #13393.
gkalpak added a commit that referenced this issue Dec 8, 2015
@gkalpak @petebacondarwin
fix($resource): don't allow using promises as timeout and log a war…
4748652 
…ning

Promises never worked correctly as values for `timeout` in `$resource`, because the same value has
to be re-used for multiple requests (and it is not possible to `angular.copy()` a promise).
Now (in addition to ignoring a non-numeric `timeout`), a warning is logged to the console using
`$log.debug()`.

Partly fixes #13393.

BREAKING CHANGE:

Possible breaking change for users who updated their code to provide a `timeout`
promise for a `$resource` request in version 1.4.8.

Up to v1.4.7 (included), using a promise as a timeout in `$resource`, would silently
fail (i.e. have no effect).

In v1.4.8, using a promise as timeout would have the (buggy) behaviour described
in #12657 (comment)
(i.e. it will work as expected for the first time you resolve the promise and will
cancel all subsequent requests after that - one has to re-create the resource
class. This is feature was not documented.)

With this change, using a promise as timeout in 1.4.9 onwsards is not allowed.
It will log a warning and ignore the timeout value.

If you need support for cancellable `$resource` actions, you should upgrade to
version 1.5 or higher.
@gkalpak
Copy link
Member Author

gkalpak commented Dec 8, 2015

Closed by #13462.

@gkalpak gkalpak closed this as completed Dec 8, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@gkalpak gkalpak

Labels
component: ngResource frequency: low severity: broken expected use type: bug type: chore type: docs
Projects
None yet
Milestone
1.4.9
Development

No branches or pull requests
1 participant
@gkalpak