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

$http does not ensure the promise returned always has success and error methods #10829

Closed
michael-schaedel opened this issue Jan 22, 2015 · 2 comments

Comments

@michael-schaedel
Copy link

Hi there, I'll start this off by stating that I do have a potential fix if the issue was not by design. The issue is that in the http class constructor we create a promise from the call to $q.when and then extend that promise with the success and error methods. This is fine, but whenever Promise.then is called, a new promise is created and we lose the success and error methods. We try to prevent this in the success and error methods by calling Promise.then and only returning back the current instance instead of the new promise generated. Unfortunately, this means that the promise returned back from an $http instantiation will only support success and error method calls if they are called prior to any standard Promise method, since all Promise method calls return a new promise. Here's a couple of examples to show the issue:

// This works as expected.
$http({ url: 'http://someurl.com' })
  .success(function () { console.log('success!'); })
  .error(function () { console.log('success!'); })
  .then(function () { console.log('then called!'); })
  .finally(function () { console.log('finally called!'); });
// This fails because any call to "then", which is called by "finally",
// returns a new promise without success and error.
$http({ url: 'http://someurl.com' })
  .finally(function () { console.log('finally called!'); })
  .success(function () { console.log('success!'); })  // success is not a function.
  .error(function () { console.log('success!'); })
  .then(function () { console.log('then called!'); });

You may ask, why is this an issue? Why would you call finally before the rest anyway, as it just visually looks incorrect and starts a fire in the belly due to your O.C.D?

A good example would be an angular service that returns the promise to the callee to handle anything post they wish, while the service itself needs to manage it's own cleanup once it's done. Consider the following:

angular.module('example', [])
  .service('Example', function ExampleService() {
    function internalCleanup() { }

    this.execute = function () {
      return $http({ url: 'http://someurl.com' }).finally(internalCleanup);
    };
  })
  .directive('Example', function ExampleDirective() {
    return {
      controller: function (Example) {
        try {
          Example.execute()
            .success(...) // success is not a function
            .error(...); // error is not a function
        } catch (e) {
          Example.execute()
            .then(
              function ExampleSuccess() { console.log('success!'); },
              function ExampleFailure() { console.log('failure!'); }
            ); // Success.
        }
      };
  });

Is this by design, or should we be supporting success and error to be called at any time, rather than being dependent on call order?

@pkozlowski-opensource
Copy link
Member

This is essentially duplicate of #4345 - see the comments in there. I would say that the sooner you switch to "real" promises the more headaches you are going to save yourself. Especially that we are discussing intention to depreciate .success and .error, see: #10508

@michael-schaedel
Copy link
Author

Been using "real" promises forever, so no sweat off my back. Glad to see that if it's not going to have complete integration that it will be removed! I've not adopted the success/error methods due to this issue, although the success/error methods do read "better". Thanks, and sorry for creating a dup ;p

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

No branches or pull requests

2 participants