feat($http) Add support for custom event hooks (progress, etc)

[prajwalkman]

In response to #1934
This is far from complete, I just wanted to open a preliminary implementation so we can discuss implementation details.
I'm neither an expert at Angular, nor in the HTTP protocol itself.

The current implementation of $http doesn't allow us to attach event handlers for anything except success and failure. There are a large number of well supported (and maybe non standard) events for XHR that are very useful, the most popular being the progress events.

This PR implements the feature by extending the config object passed to $http to add a hooks keyword, which maps events to event handler.
Example:

var configObject = {
  method: 'GET',
  url: '/specs.html',
  cache: $templateCache,
  hooks: {
    progress: function(data) {
      return $scope.progress = data;
    }
  }
};
$http(configObject).success(successHandler);

To add event handlers for upload events, nest the key mapping under a upload key:

hooks: {
  upload: {
    progress: uploadProgressHandler;
  }
}

Please suggest/discuss improvements to the syntax and/or implementation errors.
I believe the code in the PR currently works, but I haven't thoroughly tested it.

[prajwalkman]

@wesleycho @ryankshaw @mjt01 @bjconlan @aminariana

[michaeltaranto]

Thanks for the message sorry its taken me a while to respond. I think this implementation is a little bit too focused in its solution. While my particular complaint was not having the ability to hook onto the progress event, there are other complaints that could all be addressed by having access to the xhr object itself.

It seems more useful to potentially overload the $http method to return the xhr on the promise. It's already overloading it with success and error methods. Adding the xhr to the promise will allow people to abort the request, or monitor the progress, or anything else they may want to do that requires the actual request object.

Otherwise we end up adding hooks and methods for every interaction which is probably not ideal. My 2c

[prajwalkman]

Reasonably what other interaction would you have with the XHR object? I felt it had more chance of being pulled if it solved a specific problem precisely, rather than resorting to a scatter shot approach.

@IgorMinar , @mhevery Any suggestions on how we should take this forward? I feel this is a fairly basic feature to be missing, considering we're over halfway to v2.

[michaeltaranto]

The most important reason for having the xhr object I would think is aborting the request. I believe theres another thread somewhere that is interested in being able to do that. That in itself is very important to be able to stop the request.

[imccoy]

I'm just an interested party, not someone who actually knows stuff, but I'm curious about the lack of something like the $rootScope.$apply at https://github.com/prajwalkman/angular.js/blob/aa53be547c3d55214a731d68557bf2a802220be2/src/ng/http.js#L931 - I think this pull request won't work as expected without it.

I like the approach of passing a bag of event handlers in, but am curious about the special handling for the .upload object. Are there no other members of the XHR object that can have events put on them?

Have you considered adding some tests to the unit test suite for this functionality?

[prajwalkman]

@imccoy These hooks work outside the promises. Since your handlers will be in the controller itself, you don't need $rootScope.$apply. I think. I mean, I tested it and it seems to work :/

Anyway, about the unit tests, I would need to modify the HTTP Mock service, and that thing is fairly complex. I'm not even sure where to begin, or if this feature is inherently untestable.

[petebacondarwin]

Now that $q.notify has been implemented, we should close this and focus on a PR that provides access to XHR events, such as progress, via the promise returned from $http.

[benjamingr]

@petebacondarwin I humbly disagree, I think providing a hook is a much better abstraction than shoving an event emitter into the promise implementation.

See the arguments made here #1934 (comment)