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

chore($http): remove deprecated responseInterceptors functionality #7267

Closed
wants to merge 1 commit into from

Conversation

caitp
Copy link
Contributor

@caitp caitp commented Apr 27, 2014

Code cleanup! response interceptors have been deprecated for some time, and it
is confusing to have two APIs, one of which is slightly "hidden" and hard to
see, which perform the same task. The newer API is a bit cleaner and more
visible, so this is naturally preferred.

BREAKING CHANGE:

Previously, it was possible to register a response interceptor like so:

// register the interceptor as a service
$provide.factory('myHttpInterceptor', function($q, dependency1,
  dependency2) {
  return function(promise) {
    return promise.then(function(response) {
      // do something on success
      return response;
    }, function(response) {
      // do something on error
      if (canRecover(response)) {
        return responseOrNewPromise
      }
      return $q.reject(response);
    });
  }
});

$httpProvider.responseInterceptors.push('myHttpInterceptor');

Now, one must use the newer API introduced in v1.1.4 (4ae4681), like so:

$provide.factory('myHttpInterceptor', function($q) {
  return {
    response: function(response) {
      // do something on success
      return response;
    },
    responseError: function(response) {
      // do something on error
      if (canRecover(response)) {
        return responseOrNewPromise
      }
      return $q.reject(response);
    }
  };
});

$httpProvider.interceptors.push('myHttpInterceptor');

Closes #7266

@caitp caitp added this to the 1.3.0 milestone Apr 27, 2014
@petebacondarwin
Copy link
Contributor

The last line of your "correct" code example should be:

$httpProvider.interceptors.push('myHttpInterceptor');

(Noting that the property is now interceptors)

Moreover I think the commit message should be more clear about the API change from "responseInterceptors" to "interceptors", which can also intercept "requests" as well as responses. In fact part of the beauty of them is that your interceptor can wrap a full request and response pair.

@caitp
Copy link
Contributor Author

caitp commented Apr 27, 2014

yeah, thats true, i'll fix the commit msg. That API has been deprecated for a long time though

Code cleanup! response interceptors have been deprecated for some time, and it is confusing to have
two APIs, one of which is slightly "hidden" and hard to see, which perform the same task. The newer
API is a bit cleaner and more visible, so this is naturally preferred.

BREAKING CHANGE:

Previously, it was possible to register a response interceptor like so:

    // register the interceptor as a service
    $provide.factory('myHttpInterceptor', function($q, dependency1, dependency2) {
      return function(promise) {
        return promise.then(function(response) {
          // do something on success
          return response;
        }, function(response) {
          // do something on error
          if (canRecover(response)) {
            return responseOrNewPromise
          }
          return $q.reject(response);
        });
      }
    });

    $httpProvider.responseInterceptors.push('myHttpInterceptor');

Now, one must use the newer API introduced in v1.1.4 (4ae4681), like so:

    $provide.factory('myHttpInterceptor', function($q) {
      return {
        response: function(response) {
          // do something on success
          return response;
        },
        responseError: function(response) {
          // do something on error
          if (canRecover(response)) {
            return responseOrNewPromise
          }
          return $q.reject(response);
        }
      };
    });

    $httpProvider.interceptors.push('myHttpInterceptor');

More details on the new interceptors API (which has been around as of v1.1.4) can be found at
https://docs.angularjs.org/api/ng/service/$http#interceptors

Closes angular#7266
@petebacondarwin
Copy link
Contributor

LGTM

@caitp
Copy link
Contributor Author

caitp commented Apr 30, 2014

I guess if it's a chore, it won't show up in the changelog, and it belongs there. What would be a good type for this CL?

@caitp
Copy link
Contributor Author

caitp commented Apr 30, 2014

I guess it needs to be just manually added to the changelog. @btford will you be doing the next changelog update? If so can I get you to make a note to add this to it manually?

@btford
Copy link
Contributor

btford commented Apr 30, 2014

Not sure if this should be marked as a chore. :/

@caitp: yes. I think it might also be good to amend the script to include breaking changes from chores.

@btford
Copy link
Contributor

btford commented Apr 30, 2014

I verified that the changelog script does the right thing.

@caitp
Copy link
Contributor Author

caitp commented Apr 30, 2014

Okay, I agree that it probably shouldn't be marked as a chore, but I'm not sure of a better name for this type of commit.

If the changelog will show it anyways, then maybe it doesn't matter and should just be merged. SGTY?

@btford
Copy link
Contributor

btford commented Apr 30, 2014

@caitp do it

@caitp
Copy link
Contributor Author

caitp commented Apr 30, 2014

Okay, done and done

@caitp caitp closed this in ad4336f Apr 30, 2014
@btford
Copy link
Contributor

btford commented Apr 30, 2014

👍 🌟 👯

stickel pushed a commit to stickel/angular.js that referenced this pull request May 1, 2014
Code cleanup! response interceptors have been deprecated for some time, and it is confusing to have
two APIs, one of which is slightly "hidden" and hard to see, which perform the same task. The newer
API is a bit cleaner and more visible, so this is naturally preferred.

BREAKING CHANGE:

Previously, it was possible to register a response interceptor like so:

    // register the interceptor as a service
    $provide.factory('myHttpInterceptor', function($q, dependency1, dependency2) {
      return function(promise) {
        return promise.then(function(response) {
          // do something on success
          return response;
        }, function(response) {
          // do something on error
          if (canRecover(response)) {
            return responseOrNewPromise
          }
          return $q.reject(response);
        });
      }
    });

    $httpProvider.responseInterceptors.push('myHttpInterceptor');

Now, one must use the newer API introduced in v1.1.4 (4ae4681), like so:

    $provide.factory('myHttpInterceptor', function($q) {
      return {
        response: function(response) {
          // do something on success
          return response;
        },
        responseError: function(response) {
          // do something on error
          if (canRecover(response)) {
            return responseOrNewPromise
          }
          return $q.reject(response);
        }
      };
    });

    $httpProvider.interceptors.push('myHttpInterceptor');

More details on the new interceptors API (which has been around as of v1.1.4) can be found at
https://docs.angularjs.org/api/ng/service/$http#interceptors

Closes angular#7266
Closes angular#7267
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.

Remove responseInterceptors collection from $httpProvider for 1.3
3 participants