gund / ng-http-interceptor

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

ng-http-interceptor intercepting twice #135

Closed ghost closed 7 years ago

ghost commented 7 years ago

injected import { HttpInterceptorModule, HttpInterceptorService } from 'ng-http-interceptor'; imported HttpInterceptorModule.noOverrideHttp() in app.module.ts

I am using InterceptableHttp instead of http (only for few calls). I don't want to replace http for all calls. I am using import {InterceptableHttp} from 'ng-http-interceptor' in service. Now each intercepted call is happening twice. Is there a way that that intercepted call only happens once?

gund commented 7 years ago

It might be the old problem arises again after last changes where I updated the way response interceptors were called. I also removed sharing of the HTTP observable but before commit I checked that it wont duplicate requests.

I'm not sure why exactly you have duplicated requests. I will try to investigate but in the mean time can you also provide plunkr with your concrete use case?

Thanks.

gund commented 7 years ago

I just tried this plunk with updated version (2.0.3) and noOverrideHttp() and seems fine.

gund commented 7 years ago

Also just FYI: HTTP observable is so called cold by default, meaning that every subscription to it will trigger a new HTTP request. So if subscribing to http observable in response interceptor - it will eventually trigger one more http request.

theMK2k commented 7 years ago

Not sure if related, but we encountered a similar issue. We intercept every http call. With ng-http-interceptor 2.0.1 they are called once. Now with 2.0.3 they are called twice.

gund commented 7 years ago

@theMK2k you mean every HTTP request is duplicated? Can you provide some repro info or plunkr?

Thanks

theMK2k commented 7 years ago

@gund: for the weekend we'll freeze ng-http-interceptor at 2.0.1 and dig deeper starting monday. I'm sure we'll get a plunkr ready then.

gund commented 7 years ago

@theMK2k okay. Btw you can try to share http observable in your response interceptor so it should prevent multiple HTTP requests being fired:

httpInterceptorService.response().addInterceptor(res => res.share());

It will convert observable to hot and all subsequent subscribes will not trigger new http request.

UPD: It's safer to share as BehaviorSubject (in this way every subscriber will have a response):

httpInterceptorService.response().addInterceptor(res => res.publishLast().refCount());
theMK2k commented 7 years ago

@grund: thanks! We'll look into that :)

Necroskillz commented 7 years ago

Also had this issue. You can also use do operator in the response interceptor instead of subscribing.

httpInterceptorService.response().addInterceptor(res => res.do(r => { 
    // interceptor logic
}));

Maybe it would make sense to update the README to suggest using do instead of subscribe? Since then you don't need another interceptor to work as intended out of the box.

gund commented 7 years ago

@Necroskillz yes you should use do operator if you do not want to duplicate requests and it is a default behavior even if you do not use this library (just try to make 2 subscriptions to same http observable) so I do not see the reason to mention obvious behavior.

For now will close the issue since it's not a bug.

Necroskillz commented 7 years ago

I agree, only thing I would change is the example code in README

gund commented 7 years ago

Yep agree =) Will update readme thanks)