gund / ng-http-interceptor

Http Interceptor library for Angular
MIT License
104 stars 16 forks source link

version 2.0.3 leads to duplicated requests #138

Closed ujibang closed 7 years ago

ujibang commented 7 years ago

we use ng-http-interceptor to inject authentication headers and handle some http errors (such as 401 and 403)

we updated to 2.0.3 and noticed that some calls are duplicated. for instance our feature "create new stuff" leads to have 2 new rows in the db or "delete stuff" generates two DELETE calls.

downgrading to 2.0.2 fixed the issue.

ng http uses cold observables, so we wonder if 2.0.3 somehow subscribe to calls leading to this critical problem.

gund commented 7 years ago

Hi, actually there was a change from v2.0.2 to v2.0.3: before the library was sharing observable (multicasting) so that would prevent from duplicate requests and that was needed because of how interceptor chain was called but since then I reworked that calls so that it no longer requires sharing and I removed sharing making it back cold. I have tested it in various scenarios including my real life projects and did not find any duplicated requests. Not sure but maybe you have somewhere double subscription or something. If you can please provide plunker because every method I used to verify this shows no duplications.

Thanks

ujibang commented 7 years ago

I created a plunker that reproduces the issue: http://plnkr.co/edit/nODcfegVK6iXtRNgrAy6?p=preview

the plunker shows two buttons "load data" and "delete" that send GET and DELETE requests to the demo instance of RESTHeart at dbapi.io/db/coll.

loading data works fine but DELETE request gets executed twice.

The problem arises when I added the response interceptor (the request interceptor does not have this issue):

httpInterceptor.response().addInterceptor((res, method) => {
        res.subscribe(console.log('http response intercepted')); 

        return res;
      });

res.subscribe clearly leads to this issue if the res is a cold observable....

gund commented 7 years ago

Sure, .subscribe() will lead you to one more request. What you want to do probably is this:

httpInterceptor.response().addInterceptor((res, method) => {
        return res.do(() => console.log('http response intercepted'));
      });
ujibang commented 7 years ago

tried and got

EXCEPTION: Error in ./App class App - inline template:6:9 caused by: res.do is not a function

gund commented 7 years ago

Sure thing, you need to import that operator:

import "rxjs/add/observable/do"
ujibang commented 7 years ago

yep import "rxjs/add/operator/do" worked!

given that 2.0.2 and 2.0.3 manage observable in that different way, I suggest you to add a note on the README about that.

Thanks and congrats for your work!

gund commented 7 years ago

Thank you!

Actually it is the way library supposed to work in the beginning (not converting observables from "cold" to "hot") but since I was not able to get rid of duplicate requests I was forced to do that sharing.

So now it's like a natural way of working with observables as you would work with them without this library =) I'm not sure if I was mentioning that this library was sharing observables but since 2.0.3 it's not doing it - so should not bring any confusion any more =)