ngx-rocket / generator-ngx-rocket

:rocket: Extensible Angular 14+ enterprise-grade project generator
https://ngx-rocket.github.io/
MIT License
1.53k stars 217 forks source link

Circular Dependency Error Handler Interceptor Refresh Token Implementation #521

Closed TriumphTruth closed 4 years ago

TriumphTruth commented 4 years ago

I'm submitting a...

Current behavior

I am trying to catch the 401 errors in the ErrorHandlerInterceptor and based on that trying to make a call to get a new refresh token.

But doing so is resulting in circular dependency, if I use the existing HttpService / AuthenticationService / ErrorHandlerInterceptor itself.

My Error Handler


import { Injectable, OnDestroy } from '@angular/core';
import { HttpEvent, HttpInterceptor, HttpHandler, HttpRequest, HttpErrorResponse } from '@angular/common/http';
import { Observable } from 'rxjs';
import { catchError, concatMap, map, tap } from 'rxjs/operators';

import { environment } from '@env/environment';
import { Logger } from '../logger.service';
import { Router } from '@angular/router';
import { Token, UserToken } from '@app/shared/models';
import { CredentialsService } from '../authentication/credentials.service';
import { Constants } from '@app/shared/constants/constants';
import { untilDestroyed } from '../until-destroyed';
import { HttpService } from './http.service';

const log = new Logger('ErrorHandlerInterceptor');

/**
 * Adds a default error handler to all requests.
 */
@Injectable({
  providedIn: 'root'
})
export class ErrorHandlerInterceptor implements HttpInterceptor, OnDestroy {

  constructor ( public credentialSer: CredentialsService, private router: Router, private http: HttpService ) { }

  intercept(request: HttpRequest, next: HttpHandler): Observable> {
    return next.handle(request).pipe(catchError(error => this.errorHandler(error)));
  }

  // Customize the default error handler here if needed
  private errorHandler(response: HttpEvent): Observable> {

    if (!environment.production) {
      // Do something with the error
      log.error('Request error', response);
    }

    if ( response instanceof HttpErrorResponse ) {
      if ( response.status === 401 ) {
          this.refreshToken()
          .subscribe( resp => {
             if ( !resp ) {
               this.credentialSer.removeCredentials();
               this.router.navigate(['/auth/login']);
            } else {
                //implement retry of failed requests
                  //this.retryFailedRequests();
                 this.credentialSer.removeCredentials();
                 this.router.navigate(['/auth/login']);
             }
          } );
      }
  }

  if (!environment.production) {
      // Do something with the error
      log.error('Request error', response);
    }
    throw response;
  }

private refreshToken(): Observable {
  return this.credentialSer.credentials
  .pipe(
    concatMap(userDetails => 
      this.http.post(Constants.refreshToken, userDetails)
    ),
    map((response: UserToken) => response.token),
    tap(token => this.credentialSer.updateTokenCredentials(token)),
    untilDestroyed(this)
  );
}

ngOnDestroy() : void {

}

}


Now firstly I put this in the AuthenticationService. But it was giving me Circular Dependency error. Then I put it in the ErrorHandler itself.... It again gave me Circular Dependency error. The I omitted using the out of the box HttpService. And injected Angular's HttpClient. Then I get the following error:

ERROR TypeError: Cannot read property 'intercept' of undefined
    at HttpInterceptorHandler.handle (http.service.ts:42)
    at ApiPrefixInterceptor.intercept (api-prefix.interceptor.ts:26)
    at HttpInterceptorHandler.handle (http.service.ts:42)
    at MergeMapSubscriber.project (http.js:1648)
    at MergeMapSubscriber._tryNext (mergeMap.js:46)
    at MergeMapSubscriber._next (mergeMap.js:36)
    at MergeMapSubscriber.next (Subscriber.js:49)
    at Observable._subscribe (subscribeToArray.js:3)
    at Observable._trySubscribe (Observable.js:42)
    at Observable.subscribe (Observable.js:28)

Expected behavior

I should be able to make a call to get a new token.

Minimal reproduction of the problem with instructions

Environment



Angular 8
Latest starter kit

Others:

Please guide how do I implement a refresh token functionality without this circular dependency.

sinedied commented 4 years ago

See my answer here: https://github.com/ngx-rocket/generator-ngx-rocket/issues/432#issuecomment-455982764

There is many ways to avoid circular dependency issues, I suggest you to look at this article: https://medium.com/visual-development/how-to-fix-nasty-circular-dependency-issues-once-and-for-all-in-javascript-typescript-a04c987cf0de. You can also use a state management library, like https://www.ngxs.io: event-based actions design pattern is an easy way to avoid such issues.

BTW, included HttpService is provided when you use HttpClient injection token, as seen in your error stack trace. The error in that one seems to be something different.

TriumphTruth commented 4 years ago

I think what you are suggesting is to create a new service that will solely perform the, in my case Refresh Token functionality.... While it seems to be a way, will I will try and update for sure, but appears to me as a not so fancy way of bypassing this problem.

Additionally I don't think it should resolve the problem.... The problem stems from the following circular chain: 1 - ALL Services inherits HttpService 2 - Whenever we make a call using HttpService it injects the HttpInterceptors which are API, Error etc. 3 - If any of the interceptor is making a call using any service or within itself, it is again starting the same chain from step 1.

But will still try your advice and update the issue accordingly.

sinedied commented 4 years ago

FWIW, upcoming version will provides 2 answers to avoid this issue: