Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cyclic dependency error with HttpInterceptor #18224

Closed
cyrilletuzi opened this issue Jul 19, 2017 · 87 comments

Comments

@cyrilletuzi
Copy link
Contributor

commented Jul 19, 2017

I'm submitting a...


[ ] Regression (a behavior that used to work and stopped working in a new release)
[x] Bug report  
[ ] Feature request
[ ] Documentation issue or request
[ ] Support request => Please do not submit support request here, instead see https://github.com/angular/angular/blob/master/CONTRIBUTING.md#question

Current behavior

Trying to do the same example of AuthInterceptor like in official doc, to add a Authorization header on all requests.

@Injectable()
export class AuthInterceptor implements HttpInterceptor {
  constructor(private auth: AuthService) {}
 
  intercept(req: HttpRequest<any>, next: HttpHandler) {
    const authHeader = this.auth.getAuthorizationHeader();
    const authReq = req.clone({headers: req.headers.set('Authorization', authHeader)});
    return next.handle(authReq);
  }
}

Problem : with real code, the AuthService needs to request the server on login, signin, etc., so it itself inject HttpClient.

@Injectable()
export class AuthService {

  constructor(private http: HttpClient) {}

}

It creates this fatal error in console :

Uncaught Error: Provider parse errors:
Cannot instantiate cyclic dependency! InjectionToken_HTTP_INTERCEPTORS ("[ERROR ->]"): in NgModule AppModule in ./AppModule@-1:-1

Expected behavior

Should work, as it's a very common case. Otherwise interceptors are limited to use services without Http.

Environment


Angular version: 4.3.0
@tytskyi

This comment has been minimized.

Copy link

commented Jul 19, 2017

Will it work like this?

@Injectable()
export class AuthInterceptor implements HttpInterceptor {
  constructor(@Inject(forwardRef(() => AuthService)) private auth: AuthService) {}
 
  intercept(req: HttpRequest<any>, next: HttpHandler) {
    const authHeader = this.auth.getAuthorizationHeader();
    const authReq = req.clone({headers: req.headers.set('Authorization', authHeader)});
    return next.handle(authReq);
  }
}
@Toxicable

This comment has been minimized.

Copy link
Contributor

commented Jul 19, 2017

@tytskyi I tried that and didn't work

  constructor(inj: Injector) {
    this.auth = inj.get(AuthService)
  }

works though

https://plnkr.co/edit/8CpyAUSJcCiRqdZmuvt9?p=preview

@cyrilletuzi

This comment has been minimized.

Copy link
Contributor Author

commented Jul 19, 2017

I confirm that @tytskyi solution doesn't work.

@cyrilletuzi

This comment has been minimized.

Copy link
Contributor Author

commented Jul 19, 2017

For info, for now I've separated in two services, one for login/signin/... (which uses HttpClient) and one just for managing the AuthToken (no Http).

But going further, I added a error handler to catch 401 errors (when the token is invalid). But in such case, I need to call logout, so I need the service with HttpClient, and thus I fall in the initial problem again, so it is very problematic.

@alxhub

This comment has been minimized.

Copy link
Contributor

commented Jul 19, 2017

This is a true cyclic dependency, and @Toxicable's example is the correct way to handle it.

However, I'd be concerned if the AuthService was making its own HTTP requests, and AuthInterceptor added the header to every request (as it's written) without checking the details of that request. That means that a request will go through the interceptor, which goes to the auth service, which makes a request, which goes through the interceptor, and it recurses infinitely.

@alxhub alxhub closed this Jul 19, 2017

@cyrilletuzi

This comment has been minimized.

Copy link
Contributor Author

commented Jul 19, 2017

No, the AuthService contains methods which need to do some HTTP requests (like login), but the AuthInterceptor does not use them, it just needs the AuthService to get the token.

@perusopersonale

This comment has been minimized.

Copy link

commented Jul 19, 2017

My code is very similar to @cyrilletuzi, I tried @Toxicable solution, I have no more cyclic dependency but I get:

ERROR Error: Uncaught (in promise): RangeError: Maximum call stack size exceeded
RangeError: Maximum call stack size exceeded
    at _createProviderInstance$1 (core.es5.js:9480)
    ...   
@mixalistzikas

This comment has been minimized.

Copy link

commented Jul 20, 2017

I use HttpClient and I create this interceptor to add the jwt token. Everyting work perfect but I have a bad practise. I use Http inside the HttpClient Interceptor. If I change

private http: Http,
to
private http: HttpClient
I get this cycle error

Cannot instantiate cyclic dependency! InjectionToken_HTTP_INTERCEPTORS ("[ERROR ->]")
any ideas how can I make it work?

import {Injectable} from "@angular/core";
import {HttpEvent, HttpHandler, HttpInterceptor} from "@angular/common/http";
import {HttpRequest} from "@angular/common/http";
import {Observable} from "rxjs/Observable";
import {Http} from "@angular/http";
import {SiteService} from "../services/site.service";
import {Router} from "@angular/router";

@Injectable()
export class AuthInterceptor implements HttpInterceptor {

constructor(
    private http: Http,
    private router: Router,
    private siteService: SiteService
) {}

refreshToken() {
    return this.http.get(this.siteService.apiDomain() + '/api/token?token=' + localStorage.getItem('JWToken'), {})
        .map((response: any) => {
            let data = response.json();
            return {
                token: data.token,
                permissions: data.permissions,
                user: data.user,
            };
        })
}

intercept(req: HttpRequest<any>, next: HttpHandler): Observable<HttpEvent<any>> {
    const clonedRequest = req.clone({
        headers: req.headers.set('Authorization', 'Bearer ' + localStorage.getItem('JWToken'))
    });

    return next.handle(clonedRequest).catch((res) => {

        if (res.status === 401 || res.status === 403) {
            return this.refreshToken().flatMap((data) => {
                if (data.token !== '') {
                    localStorage.setItem('currentUser', JSON.stringify(data.user));
                    localStorage.setItem('currentUserPermissions', JSON.stringify(data.permissions));
                    localStorage.setItem('JWToken', data.token);
                } else {
                    localStorage.removeItem('currentUser');
                    localStorage.removeItem('currentUserPermissions');
                    localStorage.removeItem('JWToken');
                    this.router.navigate(['./auth/login']);
                    return Observable.throw(res);
                }
                const clonedRequestRepeat = req.clone({
                    headers: req.headers.set('Authorization', 'Bearer ' + localStorage.getItem('JWToken'))
                });
                return next.handle(clonedRequestRepeat);
            })
        } else {
            return Observable.throw(res);
        }

    });

}
}

Another important thing for those who will use this interceptor for their project but irrelevant to the current problem is to set headers to the refresh token response at least some seconds.

->header('Cache-Control', 'public, max-age=45')
->header('Expires', date('D, d M Y H:i:s ', time() + 45).'GMT');

@serhiisol

This comment has been minimized.

Copy link

commented Jul 20, 2017

This is interesting,

I thought that having service like AuthService, which manages token accessibility and performs login and refresh requests it's a common use case.

So basically here is the scenario. We have AuthService which depends on HttpClient (obviously). It has few methods: getAccessToken, getRefreshToken, login, refreshSession and logout.

Also in addition we need to have AuthInterceptor, which will add token to the request, if token exists. And obviously it depends on AuthService, cause service has access to the token.

So here's the problem: HttpClient depends on HTTP_INTERCEPTORS (basically it's AuthInterceptor), AuthInterceptor depends on AuthService, and AuthService depends on HttpClient. And Cyclic error.

So this common use case is not possible in the current implementation.

But why angular team couldn't implement interceptors in the similar way how the first angular interceptors work? E.g. to have extra store service, using which one we can register new interceptors, e.g. httpInterceptors.push(interceptor)?

I wrote similar library called ng4-http, which implements similar approach to the first angular. It uses additional service which stores registered interceptors. And in this case we can inject any kind of services there and use them properly. Current library uses old http module.

So the question is, am I wrong? If yes, then where ?

@raviMalireddy

This comment has been minimized.

Copy link

commented Jul 20, 2017

I have code that is quite similar to the examples posted here, and I've been encountering the same issue. In fact, it seems extremely close to what @cyrilletuzi is trying to do, I'm just trying to get the value of a token property I have set in an AuthService, I'm not trying to make use of it any other way. Just access that in the interceptor. Adding the service to the constructor in the interceptor gives me the cyclic dependency error, basically what @mixalistzikas got, down to the message, while trying the solution @Toxicable provided seems to give me the same issue @perusopersonale got, i.e. a problem with too much recursion, and the app crashing.

Eventually just decided to access the token directly from session storage where I had it stored, rather than retrieve it from the service, but the fact that this issue seems to be occurring when there doesn't seem to be any real cyclic dependency seems off. The workaround I described works for now for me, but definitely many situations where you would need to access a service somewhere, and that doesn't seem possible in this manner

@mixalistzikas

This comment has been minimized.

Copy link

commented Jul 20, 2017

This example also works for me, but i worred because http in the future may be deprecated and also is not good for the app to inject 2 different libraries for the same thing.

If anyone find a solution, please answer...

@tarasbobak

This comment has been minimized.

Copy link

commented Jul 21, 2017

As a temporary solution Auth token can be stored as a static prop on the AuthService class. Then you won't need to instantiate it.

@tarasbobak

This comment has been minimized.

Copy link

commented Jul 21, 2017

You may also try a little hack to instatiate the service, like:

constructor(private injector: Injector) {
  setTimeout(() => {
    this.authService = this.injector.get(AuthService);
  })
}

This way you won't get the max call stack exceeded error.

@perusopersonale

This comment has been minimized.

Copy link

commented Jul 21, 2017

I resolved simply not setting authService in constructor but getting in the intercept function.


  intercept(req: HttpRequest<any>, next: HttpHandler): Observable<HttpEvent<any>> {
    // Get the auth header from the service.
    const auth = this.inj.get(AuthenticationService);
    const authToken = auth.getAuthorizationToken();
    ...
   }
@mixalistzikas

This comment has been minimized.

Copy link

commented Jul 21, 2017

That's perfect @perusopersonale ..... You solve my problem...

@mixalistzikas

This comment has been minimized.

Copy link

commented Jul 21, 2017

That's the final interceptor with some logging functionality and keep loading status for loader animation

import {Injectable, Injector} from "@angular/core";
import {HttpEvent, HttpHandler, HttpInterceptor, HttpResponse} from "@angular/common/http";
import {HttpRequest} from "@angular/common/http";
import {Observable} from "rxjs/Observable";
import {SiteService} from "../services/site.service";
import {Router} from "@angular/router";
import {LoadingService} from "../../components/loading/loading.service";
import {AuthenticationService} from "../services/authentication.service";

@Injectable()
export class AuthInterceptor implements HttpInterceptor {

    constructor(private router: Router,
                private siteService: SiteService,
                private loadingService: LoadingService,
                private injector: Injector) {
    }



    private fixUrl(url: string) {
        if (url.indexOf('http://') >= 0 || url.indexOf('https://') >= 0)
            return url;
        else
            return this.siteService.apiDomain() + url;
    }

    intercept(req: HttpRequest<any>, next: HttpHandler): Observable<HttpEvent<any>> {

        const clonedRequest = req.clone({
            headers: req.headers.set('Authorization', 'Bearer ' + localStorage.getItem('JWToken')),
            url: this.fixUrl(req.url)
        });

        let authenticationService = this.injector.get(AuthenticationService);

        this.loadingService.start();
        const started = Date.now();
        return next.handle(clonedRequest)
            .do(event => {
                if (event instanceof HttpResponse) {

                    const elapsed = Date.now() - started;
                    console.log('%c Request for ' + this.fixUrl(req.urlWithParams) + ' took ' + elapsed + ' ms.', 'background: #222; color: yellow');
                }
            })
            ._finally(() => {
                this.loadingService.stop();
            })
            .catch((res) => {
                if (res.status === 401 || res.status === 403) {
                    this.loadingService.start();
                    return authenticationService.refreshToken().flatMap((data: any) => {
                        this.loadingService.stop();
                        if (data.token !== '') {
                            localStorage.setItem('currentUser', JSON.stringify(data.user));
                            localStorage.setItem('currentUserPermissions', JSON.stringify(data.permissions));
                            localStorage.setItem('JWToken', data.token);
                        } else {
                            localStorage.removeItem('currentUser');
                            localStorage.removeItem('currentUserPermissions');
                            localStorage.removeItem('JWToken');
                            this.router.navigate(['./auth/login']);
                            return Observable.throw(res);
                        }
                        let clonedRequestRepeat = req.clone({
                            headers: req.headers.set('Authorization', 'Bearer ' + localStorage.getItem('JWToken')),
                            url: this.fixUrl(req.url)
                        });
                        return next.handle(clonedRequestRepeat).do(event => {
                            if (event instanceof HttpResponse) {

                                const elapsed = Date.now() - started;
                                console.log('%c Request for ' + req.urlWithParams + ' took ' + elapsed + ' ms.', 'background: #222; color: yellow');
                            }
                        });
                    })
                } else {
                    return Observable.throw(res);
                }

            });

    }
}

And don't forget...

Another important thing for those who will use this interceptor for their project is to set headers to the refresh token response at least some seconds.

->header('Cache-Control', 'public, max-age=45')
->header('Expires', date('D, d M Y H:i:s ', time() + 45).'GMT');

Also...
And this is my loadingService


@Injectable()
export class LoadingService {

    public count = 0;

    constructor() { }

    start(): void {
        this.count++;
    }

    stop(): void {
        this.count--;
    }


}
@nazarkryp

This comment has been minimized.

Copy link

commented Jul 22, 2017

Just to remind if someone does not know. RefreshToken can be retrieved only while main token is not expired. You can call token endpoint and post refresh token to obtain new accessToken (e.g. 1 hour before main token os expired).The problem is the following. In such case our method getAccessToken() will return observable result because it exposes http end obtaines new token using refresh token if necessary.
So how would you handle the following: handle cyclic dependency issue and inject observable to interceptor?

@alxhub

This comment has been minimized.

Copy link
Contributor

commented Jul 24, 2017

@perusopersonale that stack overflow error means that your circular dependency has been resolved at build time but is still very much present at runtime, you're making a new request from within the interceptor which also goes through the interceptor, and that repeats forever.

I suggest using next.handle() to make the dependent request instead of trying to get ahold of HttpClient there.

@SirZach

This comment has been minimized.

Copy link

commented Aug 3, 2017

I want to inject a service for the purpose of getting a value stored out of the service, not to make a request, but that service does make use of the HttpClient to make requests. I don't think this issue should be closed.

@nazarkryp

This comment has been minimized.

Copy link

commented Aug 4, 2017

@SirZach

This comment has been minimized.

Copy link

commented Aug 4, 2017

@nazarkryp when using the manual injection method, I seem to be getting a different instance of the service. That could be my fault with how I set up the provider for the service (although I think I took care of only providing it once). Are you seeing the same thing?

@will-copperleaf

This comment has been minimized.

Copy link

commented Aug 15, 2017

I'm having the same issue, and seems there's still no satisfying answer provided above. This shouldn't be closed.

@themainframe

This comment has been minimized.

Copy link

commented Aug 23, 2017

Agreed @will-copperleaf. I still view this as a design problem with HttpClient/Interceptors in general. Interceptors needing to make requests themselves is a fairly common case (particularly when they're being used for authentication, as I suspect is the case for most here). Injecting the Injector and requesting services ad-hoc seems a bit hacky.

@papaiatis

This comment has been minimized.

Copy link

commented Sep 6, 2017

Getting an Injector instance does not work for me.

Error:

AppComponent_Host.html:1 ERROR TypeError: Cannot set property 'injector' of undefined
    at AuthHttp (authhttp.interceptor.ts:10)
    at eval (module.ngfactory.js? [sm]:1)
    at _callFactory (core.es5.js:9550)
    at _createProviderInstance$1 (core.es5.js:9489)
    at resolveNgModuleDep (core.es5.js:9471)
    at _callFactory (core.es5.js:9550)
    at _createProviderInstance$1 (core.es5.js:9489)
    at resolveNgModuleDep (core.es5.js:9471)
    at _createClass (core.es5.js:9514)
    at _createProviderInstance$1 (core.es5.js:9486)

AuthHttp class:

1.  import { HttpInterceptor, HttpRequest, HttpHandler, HttpEvent } from '@angular/common/http';
2.  import { Injectable, Injector } from '@angular/core';
3.  import { Observable } from 'rxjs/Rx';
4.  import { AuthService } from './auth.service';
5.
6.  @Injectable()
7.  export class AuthHttp implements HttpInterceptor {
8.     constructor(private injector: Injector) { }
9.
10.    intercept(req: HttpRequest<any>, next: HttpHandler): Observable<HttpEvent<any>> {
11.        return next.handle(req);
12.    }
13.  }

@alxhub alxhub reopened this Sep 7, 2017

@MarkPieszak

This comment has been minimized.

Copy link
Member

commented Sep 16, 2017

@will-copperleaf Which example have you tried?

You should be able to do this for example:

import { Injectable, Injector } from '@angular/core';
import { HttpInterceptor, HttpEvent, HttpRequest, HttpHandler } from '@angular/common/http';
import { Observable } from 'rxjs/Observable';
import { AuthService } from '../shared/auth.service';

@Injectable()
export class AuthInterceptor implements HttpInterceptor {

  private authService: AuthService;

  constructor(
    private injector: Injector
  ) { }

  intercept(request: HttpRequest<any>, next: HttpHandler): Observable<HttpEvent<any>> {

    this.authService = this.injector.get(AuthService); // get it here within intercept

    const authRequest = request.clone({
      headers: request.headers.set('Authorization', this.authService.getHeaders())
    });

    return next.handle(authRequest);
  }

}
@klikas

This comment has been minimized.

Copy link

commented Sep 26, 2017

So basically if my authService needs to do an http request to retrieve both the token and the refresh token and then store those in localStorage, how would you propose to implement this?
I see in the above examples that people inside the intercept method add the headers from the authService or directly add the token from localStorage(which means you already have a token).
What if my getHeaders() method is asynchronous?

From what I understand, there is no way to exclude specific requests from the interceptors so that they don't get intercepted, there is no way to load the interceptors for a specific component or module (they have to be defined in the app module).

@twoheaded

This comment has been minimized.

Copy link

commented Oct 2, 2017

I use this solution in my auth.interceptor.ts file:

intercept(req: HttpRequest<any>, next: HttpHandler): Observable<HttpEvent<any>> {

  if (req.url.includes('/token')) {
    return next.handle(req);
  }
  this.tokenService = this.injector.get(TokenService);
...
}
@MrCroft

This comment has been minimized.

Copy link

commented Feb 1, 2018

I see this was closed with the release of 5.2.3 (and it is mentioned in the changelog).
But it still doesn't work.
I've just updated to 5.2.3 (nuked node_modules and package-lock.json first)
Then I've replaced the old code in my interceptor:

  constructor(
    private injector: Injector
  ) {}

  intercept(req: HttpRequest<any>, next: HttpHandler): Observable<HttpEvent<any>> {
    const authService = this.injector.get(AuthService);
    const token = authService.getToken();
    const reqCopy = req.clone({
      headers: req.headers.append('Authorization', `Bearer ${token}`)
    });
    return next.handle(reqCopy);
  }

with what it should be:

  constructor(
    private authService: AuthService
  ) {}

  intercept(req: HttpRequest<any>, next: HttpHandler): Observable<HttpEvent<any>> {
    const token = this.authService.getToken();
    const reqCopy = req.clone({
      headers: req.headers.append('Authorization', `Bearer ${token}`)
    });
    return next.handle(reqCopy);
  }

And I get the cyclic dependency error when I try to build with the --prod flag.

@melicerte

This comment has been minimized.

Copy link

commented Feb 5, 2018

@MrCroft I just updated to angular 5.2.3, it is working just fine.
yarn upgrade --scope @angular
Maybe there is something more causing the problem in your case.

@afvicente

This comment has been minimized.

Copy link

commented Feb 6, 2018

I also get the cyclic dependency error when I try to build with the --prod flag.

@jzahoor

This comment has been minimized.

Copy link

commented Feb 10, 2018

I was getting the same cyclic error when using angular 5.2.2.
Ran npm update in Visual Studio Code and that updated to 5.2.4. No more error. Seems like the Angular team addressed the issue with the latest release.

@bogomips

This comment has been minimized.

Copy link

commented Feb 10, 2018

@jzahoor I'd like to know more about the fix.
My work around (many months ago) was using "fetch" inside the interceptor in order to completely bypass angular...
Thanks

@MrCroft

This comment has been minimized.

Copy link

commented Feb 10, 2018

@melicerte sorry for the late response, didn't realize I had to pay my hosting provider. It was weird not getting email notifications for a few days, until I realized why :)

On topic: It turns out, if you have a Universal app, you still get the error. Of course I didn't notice this until I decided to create a new app, from scratch 😄
I don't even have to build the Universal app, it just needs to be there.
Simply running ng serve --prod or ng build --prod throws the error (this builds the first app in the apps array, which is the client in my case).

@alxhub I've made a repo here to reproduce it.
Simply running npm install and ng serve --prod (or ng build --prod to build the client app) will produce the error.
Weird thing is that it will mention app.server.module, even though you're just building the client app.
Not sure if this is an issue for the CLI rather than Angular?

Angular CLI: 1.6.8
Angular: 5.2.4
Node: 8.9.4
OS: win32 x64

@jdhines

This comment has been minimized.

Copy link

commented Feb 15, 2018

@marcincichocki: interested to know how MY_REQUEST is being sent to your interceptor. I like your solution of just bypassing interceptors for certain things like auth, just not sure how to use your example for my existing interceptor.

@marcincichocki

This comment has been minimized.

Copy link

commented Feb 15, 2018

@jdhines In that particular example you would need to provide MY_REQUEST token via dependency injection:

@NgModule({
  providers: [
    {
      provide: MY_REQUEST,
      useValue: new HttpRequest() // set your HttpRequest here, or use factory function if you need DI
    }
  ]
})

However this is good if you need highly portable and reusable code(let's say you have multiple apps that use different backends). I you don't, just set HttpRequest as property on interceptor itself:

@Injectable()
export class MyInterceptor  {
  myRequest = new HttpRequest()
}
@jdhines

This comment has been minimized.

Copy link

commented Feb 15, 2018

@marcincichocki: thanks (and apologies for the thread here). Pardon what's probably a basic question; this is all pretty foreign to me, but is myRequest the backend stand-in for what is http.get('userInfo') in my auth-service? I assume you'd actually pass something to new HttpRequest()? If so, what?

@marcincichocki

This comment has been minimized.

Copy link

commented Feb 15, 2018

@jdhines Yes, you must pass arguments describing your request. That class have few overloads

constructor(method: 'DELETE'|'GET'|'HEAD'|'JSONP'|'OPTIONS', url: string, init?: {
headers?: HttpHeaders,
reportProgress?: boolean,
params?: HttpParams,
responseType?: 'arraybuffer'|'blob'|'json'|'text',
withCredentials?: boolean,
});
constructor(method: 'POST'|'PUT'|'PATCH', url: string, body: T|null, init?: {
headers?: HttpHeaders,
reportProgress?: boolean,
params?: HttpParams,
responseType?: 'arraybuffer'|'blob'|'json'|'text',
withCredentials?: boolean,
});
constructor(method: string, url: string, body: T|null, init?: {
headers?: HttpHeaders,
reportProgress?: boolean,
params?: HttpParams,
responseType?: 'arraybuffer'|'blob'|'json'|'text',
withCredentials?: boolean,
});
constructor(
method: string, readonly url: string, third?: T|{
headers?: HttpHeaders,
reportProgress?: boolean,
params?: HttpParams,
responseType?: 'arraybuffer'|'blob'|'json'|'text',
withCredentials?: boolean,
}|null,
fourth?: {
headers?: HttpHeaders,
reportProgress?: boolean,
params?: HttpParams,
responseType?: 'arraybuffer'|'blob'|'json'|'text',
withCredentials?: boolean,
}) {

Most commonly you would use something like this:

const myRequest = new HttpRequest('POST', 'url/here', payload, options) // post
const myRequest2 = new HttpRequest('GET', 'url/here', options) // get

jbogarthyde added a commit to jbogarthyde/angular that referenced this issue Feb 23, 2018

fix(common): allow HttpInterceptors to inject HttpClient (angular#19809)
Previously, an interceptor attempting to inject HttpClient directly
would receive a circular dependency error, as HttpClient was
constructed via a factory which injected the interceptor instances.
Users want to inject HttpClient into interceptors to make supporting
requests (ex: to retrieve an authentication token). Currently this is
only possible by injecting the Injector and using it to resolve
HttpClient at request time.

Either HttpClient or the user has to deal specially with the circular
dependency. This change moves that responsibility into HttpClient
itself. By utilizing a new class HttpInterceptingHandler which lazily
loads the set of interceptors at request time, it's possible to inject
HttpClient directly into interceptors as construction of HttpClient no
longer requires the interceptor chain to be constructed.

Fixes angular#18224.

PR Close angular#19809
@danielalvenstrand

This comment has been minimized.

Copy link

commented Mar 8, 2018

If you have this problem on Ionic, you might find yourself struggling with Maximum Call Stack Size Exceeded. Just do the inj.get(...) inside platform.ready().then(() => [...]); inside of the Interceptor!

@stephaneeybert

This comment has been minimized.

Copy link

commented Mar 12, 2018

I got stuck with a similar cyclic dependency and resorted to do the workaround of manually injecting the service https://stackoverflow.com/questions/49240232/getting-a-cyclic-dependency-error

@thiagodamico

This comment has been minimized.

Copy link

commented Mar 13, 2018

I still have this problem, anyone has soved anyway?
I'm trying create a interceptor for include an Authorization in my Header.

The error: ERROR Error: Uncaught (in promise): RangeError: Maximum call stack size exceeded

My code:

intercept(request: HttpRequest<any>, next: HttpHandler): Observable<HttpEvent<any>> {
    let tokenProvider = <TokenProvider>this.injector.get(TokenProvider);

    return Observable.create((observer: any) => {
      setTimeout(() => {
        observer.next(tokenProvider.getAsyncToken());
        observer.complete();
      });
    }).mergeMap((token) => {
      if(token){
        request = request.clone({
          setHeaders: {
            Authorization: `Bearer ${token}`
          }
        });
      }
      return next.handle(request);
    });
  }

My TokenProvider:

async getAsyncToken(): Promise<any> {
    return new Promise((resolve) => {
      let token = localStorage.getItem('token');
      let time = Math.round(+new Date() / 1000);
      if(!token){
        this.http.post<Token>(`${this.urlApi}/${this.urlAuth}`, {timestamp: time}).subscribe(data => {
          resolve(data.token);
        }, err => {
          console.log(err);
        });
      }else{
        resolve(token);
      }
    });
  }
@stephaneeybert

This comment has been minimized.

Copy link

commented Mar 14, 2018

@thiagodamico I now have the exact same error message as you and I don't think I have touched anything in my source code.

@jcmmv00

This comment has been minimized.

Copy link

commented Mar 16, 2018

Me too.

leo6104 added a commit to leo6104/angular that referenced this issue Mar 25, 2018

fix(common): allow HttpInterceptors to inject HttpClient (angular#19809)
Previously, an interceptor attempting to inject HttpClient directly
would receive a circular dependency error, as HttpClient was
constructed via a factory which injected the interceptor instances.
Users want to inject HttpClient into interceptors to make supporting
requests (ex: to retrieve an authentication token). Currently this is
only possible by injecting the Injector and using it to resolve
HttpClient at request time.

Either HttpClient or the user has to deal specially with the circular
dependency. This change moves that responsibility into HttpClient
itself. By utilizing a new class HttpInterceptingHandler which lazily
loads the set of interceptors at request time, it's possible to inject
HttpClient directly into interceptors as construction of HttpClient no
longer requires the interceptor chain to be constructed.

Fixes angular#18224.

PR Close angular#19809
@simeyla

This comment has been minimized.

Copy link

commented May 4, 2018

I'm not sure why noone else has pointed this out but apparently this is fixed for Angular 6 by means of introducing an HttpInterceptingHandler which itself lazily loads the interceptors and avoids the circular dependency.

@ronen1malka

This comment has been minimized.

Copy link

commented May 13, 2018

Any example for HttpInterceptingHandler usage?

@stephaneeybert

This comment has been minimized.

Copy link

commented May 20, 2018

@ronen1malka You are not supposed to use this class as it is not part of the public API. Simply upgrade to the 6.0.2 version and keep using the HttpInterceptor class.

@andremarcondesteixeira

This comment has been minimized.

Copy link

commented Jun 11, 2018

@simeyla pointed out about HttpInterceptorHandler, good point!
But I wonder, why did the Angular team decided to create yet another concrete class to solve cyclic dependency problems instead of just adding an interface with the HttpClient signature

@tkssharma

This comment has been minimized.

Copy link

commented Jun 20, 2018

the bad part is something not working upgrade it !! lot of dirt being pushed on NPM

@marvin-SL

This comment has been minimized.

Copy link

commented Sep 19, 2018

If you have this problem on Ionic, you might find yourself struggling with Maximum Call Stack Size Exceeded. Just do the inj.get(...) inside platform.ready().then(() => [...]); inside of the Interceptor!

Works with ionic 3 and angular 5

@Sarfarazsajjad

This comment has been minimized.

Copy link

commented Nov 5, 2018

got same issue on angular 6 but solved with this method #18224 (comment)

@rdkmaster

This comment has been minimized.

Copy link
Contributor

commented Nov 29, 2018

@Toxicable thank you, your solution works for me!

I got the same issue under angular 5.0.x, but it seems that the issue is fixed under angular 5.2.11. @Toxicable 's sulotion works under both 5.0.x and 5.2.11

@ArsalanSavand

This comment has been minimized.

Copy link

commented Mar 5, 2019

This also works:

import { Injectable } from '@angular/core';
import { HttpErrorResponse, HttpEvent, HttpHandler, HttpInterceptor, HttpRequest } from '@angular/common/http';
import { Observable, throwError } from 'rxjs';
import { catchError } from 'rxjs/operators';
import { AuthService } from '../auth/auth.service';

@Injectable({
  providedIn: 'root'
})
export class JwtInterceptorService implements HttpInterceptor {

  constructor(private authService: AuthService) {
  }

  intercept(request: HttpRequest<any>, next: HttpHandler): Observable<HttpEvent<any>> {
    // Add authorization header with bearer token if available.
    const token: string = this.authService.tokenValue;
    if (token) {
      request = request.clone({
        setHeaders: {
          Authorization: `Bearer ${token}`
        }
      });
    }

    return next.handle(request).pipe(catchError((error: HttpErrorResponse) => {
      // Auto logout if 401 response returned from api
      if (error.status === 401) {
        this.authService.unAuthUser();
      }
      return throwError(error);
    }));
  }
}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
You can’t perform that action at this time.