search

tomastrajan / angular-ngrx-material-starter

Angular, NgRx, Angular CLI & Angular Material Starter Project
https://tomastrajan.github.io/angular-ngrx-material-starter
MIT License
2.82k stars 919 forks source link

HttpErrorResponse are handled twice in HttpInterceptor and Errorhandler #534

Closed chan-dev closed 3 years ago

chan-dev commented 3 years ago

Don't you think it get's handled in two places?

https://github.com/tomastrajan/angular-ngrx-material-starter/blob/0bdf1e5b9d415b2e3007bd1d6268e70c70205552/projects/angular-ngrx-material-starter/src/app/core/http-interceptors/http-error.interceptor.ts#L25

My question is why not just use throwError inside the interceptor to propagate the error back to the app?

tomastrajan commented 3 years ago

As far as I remember, this is an side effect for errors in backend request so we display some notification but then the error is propagated to the call site where the request was made which usualy then handles error directly.

throwError would do nothing in this case as tap operator is only for side effects and does not influence the origin stream.

Hope that clarifies your question.

chan-dev commented 3 years ago

It was a mistake on my part. What I mean is instead of tap we use catchError and rethrow the error back using throwError. With the current setup isn't the notification service gets called twice since the interceptor just delegates the handling to ErrorHandler, and then the propagated error also will once again be handled by ErrorHandler. maybe I'm wrong but that's how I feel like how it goes.

tomastrajan commented 3 years ago

@chan-dev it would be, but the assumption is that every called handles its own erros and converts them to the valid value so they never buble up to central error handler.

chan-dev commented 3 years ago

got ya, i guess we should add it in the documentation or README?

tomastrajan commented 3 years ago

@chan-dev that could be valuable addition ;)