ngResource interceptors called inconsistently

[jrm-airwatch]

Action-specific interceptors defined in a $resource call are called inconsistently. A response interceptor is called before the success callback, but a responseError interceptor is called after the error callback. Further, the success callback is called even if the response interceptor rejects, and the error callback is called even if the responseError interceptor resolves. And response data is populated before any interceptors run.

This behavior runs contrary to the expectation that an interceptor should transparently intercept results before passing them to the consumer. A more sensible flow would be

http success -> interceptor.response (resolves) -> populate results -> success callback
http success -> interceptor.response (rejects) -> error callback
http error -> interceptor.responseError (resolves) -> populate results -> success callback
http error -> interceptor.responseError (rejects) -> error callback

As it stands, even if a responseError interceptor successfully retries, the error callback is still called, and no response data is populated!

plunker

[nicole-ashley]

This is quite a frustrating issue. If it helps, here is a plunker that demonstrates the responseError issues. Check the console logs to see the issue.

Note that:

• interceptors are called after callbacks
• modified payloads (both content and success/rejection) are ignored

[gkalpak]

Indeed the concept of interceptors in the context of $resource is very vague. In the documentation, it is implied that interceptors are essentially $http interceptors, but they are used quite differently. Furthermore, their behavior is inconsistent and not well defined.

Similar issues have been discussed in #11409, as well.
IMO, $resource interceptors should be behave like this:

• They should be applied to the promise returned by the call to $http (before anything $resource-specific).
• The appropriate $resource interceptor should be applied (if specified by the user). Interceptors should be passed the response - nothing $resource-specific yet.
• If the interceptor rejects, the error callback should be called (and the data should not be updated).
• If the interceptor resolves, $resource should update the returned instance based on the content of response.data and call the success callback.

Thoughts anyone ?

[krotscheck]

I overall agree with the original issue description and @gkalpak's assessment. Also, I'm making the assumption that transformRequest and transformResponse both execute before/after the interceptors are considered, so I'm deliberately not including them.

Errors on response interceptors aren't the only problem - request interceptors are not actually supported by $resource at this time (see my patch), and introducing them makes the resolution of request interceptor errors tricky. If an $http request interceptor errors, which handler in $resource should get it? Since you've already passed down to the $http level, I can argue that the request has already been handed off, and everything should be treated as a response (i.e. $resource.interceptor.responseError). Alternatively, I can argue that since the error occurred during request preprocessing, it's still a request and therefore should be handled by $resource.interceptor.requestError.

My mental model of this is basically a hole. At the bottom of the hole is your response data, and you can only climb down the hole far enough until you get your first error. After that, you have to climb back up through the error interceptors instead of the response interceptors. Thus I'm more in favor of the "Everything from $http is considered a response error" way of thinking of the problem, which from what I understand is how it works now. I'd have to dig into it a bit more to see how the responseError headers and data are constructed to see if that makes sense.

[gkalpak]

@krotscheck, good points raised. I too lean towards treating anything coming back from the call to $http as "response" (we wouldn't be able to treat is otherwise without hacking $http's behavior too and that's something I'd like to avoid).

In a sense they are not request interceptors (as those in $http's config), they are more like "$http interceptors", i.e. intercepting the call to $http. At this point, I don't think it makes sense to change names (e.g. preHttp/preHttpError/postHttp/postHttpError*), as it would probably be more confusing to developers than helpful.

It's important to point out the bahavior in the documentation, though.

_{(*: Yeah, I know, I'm not good with names...)}

[krotscheck]

One more thing I discovered: requestInterceptors on $http are arrays of methods. For $resource they're individual methods. Probably something we should update.