feat($http): pass success flag to transformResponse

[just-boris]

Request Type: feature

How to reproduce:

Component(s): $http

Impact: small

Complexity: small

This issue is related to:

Detailed Description:

Other Comments:

Now transformResponse doesn't know about request is successful or not.
So, it is impossible to make some transformation only on success response. For example, I wrap response into object for easier handling:

function Post(data) {
  this.author = data.user.name;
  this.annotation = data.content.split('\n')[0];
}

$http.get('/posts', {
  transformResponse: function transformResponse(data) {
    return new Post(data)
  }
});

When request has failed, response doesn't contain a fields required by the post and it causes an Exeption.
With this fix I will can create the post only from success responses

$http.get('/posts', {
  transformResponse: function transformResponse(data, headers, success) {
    return success ? new Post(data) : data
  }
});

[mary-poppins]

Thanks for the PR! Please check the items below to help us merge this faster. See the contributing docs for more information.

• Uses the issue template (#6734)

If you need to make changes to your pull request, you can update the commit with git commit --amend.
Then, update the pull request with git push -f.

Thanks again for your help!

[pkozlowski-opensource]

Hi @just-boris , thnx for this pull request. I'm kind of the fence when looking on this one, since I'm not sure how common this use-case is and it could be already covered with interceptors, if I'm not mistaken. The transform function should be probably getting the whole request / response object in the first place, but then it is even closer to interceptors. So yes, maybe we should just stick to interceptors... Would it work for your use-case?

If others weight to merge it, it would require doc update here:

angular.js/src/ng/http.js

Line 545 in 8d0cb30

* - **transformResponse** –

[lgalfaso]

@pkozlowski-opensource you can define a different transformResponse per call while the interceptors are global, so I think this does add some value. Anyhow, I am somehow worried about the fact that the example posted #6734 (comment) does not work as it expects the default transformations to be executed before the request specific transformation

[gabzim]

This feature would be really useful. transformResponse is all about mutating the response received from the server. Now I don't think it is a rare case to mutate it depending on the success or failure of the request. I mean we already get access to all the headers in the second param, it is a little strange that we can't access the status of the response itself.

[cfclrk]

I would find this feature very useful. This PR does seem to address an intended use case. According to the documentation, transformResponse is typically meant to deserialize a response, and $http interceptors are used for things like authentication.

However using an interceptor for authentication and transformResponse for deserialization doesn't work for the reasons mentioned by the OP (an un-authenticated request resulting in a 401 will be deserialized before the authentication interceptor intercepts the response). So either interceptors need to run before transformResponse (why don't they?) or transformResponse needs to be able to choose when to run.

[Yankovsky]

+1

[Hypercubed]

This is old, but... +1, I had expected that transformResponse would only be used if the response was a success.

[excentric]

+1

[pkozlowski-opensource]

We should land this one, it solves #10324 as well

[pkozlowski-opensource]

@just-boris I know we were slow on this one, I'm sorry for this. We want to land this one - would you mind rebasing it on top of the current master?

[pkozlowski-opensource]

I'm going to put into the 1.4 milestone as I think we should seriously consider not invoking response transform functions in case of error. Would be great to have more feedback on this from people but yeh, for now I'm leaning towards removing invocation of transform functions for failed responses.

[excentric]

Agreed. An error response very rarely, and never in my use cases to date, is transformed in the same way a success response is. A separate transform function for errors might work.

The key here being: 'transformed the same way'

[caitp]

I actually disagree with this line of thinking --- If I request "/me" and get a 403, that 403 might have information in JSON that I care about.

a website will render HTML on a 404, we should still transform error responses. The only case where it doesn't make a lot of sense is the NetworkError case, but we can't really do that because mobile apps sometimes send responses that look like NetworkErrors for valid responses.

[pkozlowski-opensource]

Yeh... Still, people often need to know (for whatever reason) if a given response was success / error. So we've got several options here:

• pass a success / error flag to the transform response function
• pass the whole response object to the transform response function
• have a separate functions for success / error

What would you suggest @caitp

[caitp]

I think we can return the status code to transformResponse, that's probably good enough

[pkozlowski-opensource]

OK then, let's try this approach in 1.3.x and if anything else bad surfaces we can take more radical steps in 1.4

[just-boris]

good!
@pkozlowski-opensource is there changes between my proposing API and yours?