feat($http): add XHR events configuration option

[chrisirhc]

Allows for arbitrary event handling on requests.

There are no tests here, it's just a proposal for discussion of #1934 .
I'll add tests if a decision is made to go ahead with it.
This may also simplify testing by just firing events on the mockXhr object.

[pkozlowski-opensource]

@chrisirhc I don't think is right - be doing so we would allow people to override already added (by Angular itself) handlers, no?

I know that this change would make things super-flexible but this doesn't seem to be in-line with the current API.

Do you have any other use-cases (besides download / upload events) on your mind for this one?

[chrisirhc]

@pkozlowski-opensource This doesn't override existing handlers, addEventListener always adds on existing listeners. In fact, this is safer than allowing users to do anything to the onreadystatechange attribute or having raw access to the XHR object. See example in HTML spec: https://html.spec.whatwg.org/multipage/webappapis.html#events

Use Case: I'm streaming a response and want to read the incomplete response as it is being streamed. Think of a timeseries chart that renders live as the data is coming in.

[chrisirhc]

@pkozlowski-opensource Ping. I saw that perhaps you put this in Purgatory because you thought that it would allow people to override the handlers, but it won't.

As for the being in line with the API, it would be interesting to design an API that's in line that adds such functionality. I probably need to study more API design to come up with something better.

[petebacondarwin]

@pkozlowski-opensource:
I think that something like this might be quite nice, although I wonder if we would need to include $apply in there somewhere.

What are the downsides to this?

[gkalpak]

Why put listeners inside upload if it's not going to make any difference ?

[petebacondarwin]

?

The eventHandlers object has the following structure:

{
  someEvent: ...,
  someOtherEvent: ...,
  upload: {
    someUploadEvent: ...,
    someOtherUploadEvent: ...
  }
}

[gkalpak]

Yes, but we end up doing xhr.addEventListener(key, value) for all of them.
I.e. I could write you example as:

{
  someUploadEvent: ...,
  someOtherUploadEvent: ...
  upload: {
    someEvent: ...,
    someOtherEvent: ...,
  }
}

// or

{
  someEvent: ...,
  someOtherEvent: ...,
  someUploadEvent: ...,
  someOtherUploadEvent: ...
}

and the result would be the same.

So, I am wondering if there is any benefit in separating into two categories. Why not keep it flat ?

[chrisirhc]

Sorry, this is a typo on my part. This is meant to attach events on xhr.upload. The line 100 should read xhr.upload.addEventListener(key, value);.

I'll be very happy to fix this if there's any interest in the PR.

Specifically, this is because there's progress events for both upload and the xhr object itself.

[gkalpak]

Makes sense now; I didn't know about xhr.upload 😕

[chrisirhc]

Updated PR to correct typo (xhr.upload).

[petebacondarwin]

OK, so all we need now are some unit tests and docs then this is good to go.

[chrisirhc]

• Added a unit test
• Added brief docs in $http

[petebacondarwin]

LGTM - @pkozlowski-opensource ?

[gkalpak]

Since we are mocking the addition of event listeners, would it make sense to also mock emitting some event (so that people can test their code once it relies on a callback to be called on event X) ?
Would it be possible in a clean and reliable way without a breaking change ?

[chrisirhc]

That makes sense and could be implemented like MockWindow's fire method .
However, I don't want to put too many changes in this PR at the moment as it'll affect reviewing, and the chances of it getting merged.

[mchambaud]

This is not in 1.5.0-RC.0?

[petebacondarwin]

Sorry, @mchambaud, it didn't make the cut for 1.5 - but worry not, 1.6 will be coming in not too far behind 1.5, which should be released early after the New Year.

[mchambaud]

@petebacondarwin Perhaps the milestone should be updated!

[petebacondarwin]

Thanks @mchambaud - I have updated the milestone :-)

[mchambaud]

👍

[marcelrouw]

When will 1.6 be released?
Or can this item still be added to 1.5.1 release?

[petebacondarwin]

All non-breaking changes will be included in 1.5.x

[marcelrouw]

May be a stupid question. But is this a breaking change?

[petebacondarwin]

I don't think it is a BC

[petebacondarwin]

Let's get this into 1.5.x (probably 2 or 3)

[buraktamturk]

This isn't merged yet... And I don't think it is gonna be. I've been waiting a such feature on Angular for years, yet it is 2016 and we still can't track upload with $http.

I found a pretty good workaround somewhere in the web, before this code, I were using XHR directly, using XHR directly has a few disadvantages:

• I weren't able to refresh the token if 401 returns from the server. All of the token refreshing mechanism were in custom service $api wrapped around $http.
• the service $api was including token in the request, with using XHR directly, I had manually get the token and call setRequestHeader etc. There would two authentication mechanism and it is bad for a project structure.
• it is just ugly.

It was good idea to keep $http / $api logic in my project.

here is an example

// XHR decorators were recently added to angular.

/* app */.decorator("$xhrFactory", function($delegate, $rootScope) {
    'ngInject';

    return function(method, url) {
        var xhr = $delegate(method, url);

        xhr.setRequestHeader = (function(sup) {
            return function(header, value) {
                if ((header === "__XHR__") && angular.isFunction(value))
                    value(this);
                else
                    sup.apply(this, arguments);
            };
        })(xhr.setRequestHeader);

        return xhr;
    };
});

// somewhere in the project

$http({
  method: 'PUT',
  url: `files/user/private/${file.name}`,
  data: 'the file object',
  headers: {
      'Content-Type': 'application/blablabla',

      __XHR__: function() {
          return function(xhr) {
              // here you can access the XHR
              xhr.upload.addEventListener("progress", function(event) {
                  upload_callback(event.loaded, event.total);
              });
          };
      }
  }
});

It would be much better if we could track the upload without hooking setRequestHeader. But i think it is the best solution to use something like until this PR get merged.

[chrisirhc]

Is this blocked on me rebasing this PR / making it merge-able? If it is, I can look into it.

[petebacondarwin]

I think we are OK, it just got lost in the huge pile of papers in the in-tray

[buraktamturk]

@petebacondarwin Are there any update on this?

edit: didn't see the commits, sorry.

[petebacondarwin]

I have a pull request here that should land soon #14367