Keycloak-bearer.interceptor running keycloak-js calls inside 'angular' zone

[JohnSurina]

Bug Report or Feature Request (mark with an x)

- [x ] bug report -> please search for issues before submitting
- [ ] feature request

Versions.

keycloak-angular : 12.2.0 angular: 14.1.0 keycloak-js: 12.2.0

Issue Description

Hey team, I have an angular application which features an "automatic loading spinner", this spinner is currently implemented to listen to periodically check NgZone.hasPendingMacrotasks, and once the zone is clear of tasks, unshow the spinner.

What my team does to accomplish this, in general, is to wrap all 3rd party library calls in NgZone.runOutsideAngular(<3rdPartyCall>). This allows us to have fairly tight control over our zone, and we can trust the "hasPendingMacrotasks" variable to mean "all loading logic is done" if it's false.

The issue we've run into, is that the HttpInterceptor (KeycloakBearerInterceptor) that's included in your module has a couple of calls that eventually make it's way to keycloak.js, and inside of keycloak.js (specifically line 998 in this file, contained in the setToken method; however I haven't checked to see if there are other calls that qualify as "macrotasks") there are calls to builtin apis that count as "MacroTasks" (see this primer document on zone.js for details, and this github doc page for details on what is by default considered which type of zone task.

So what I'm requesting be done, is to wrap all calls to the keycloak-service in the KeycloakBearerInterceptor file with NgZone.runOutsideAngular(). I've run a couple of rudimentary tests in my own application to ensure if a dependee of this library opts out of zone, that this call wouldn't throw an error.

I'm willing to take on the little changes and test them in this codebase myself, and submit a PR in the next day or so, assuming, of course, the maintainers of this library don't have any issues with it.

For reference, a simplified example of what my team is doing follows:

setAutoLoadingSpinner(): void {
    this.zone.runOutsideAngular( () => {
        interval(10)
            .pipe(
                .tap(() => this._isLoading = true)
                .map(() => !this.zone.hasPendingMacrotasks)
                .filter((stable) => !!stable )
                .take(1)
            ).subscribe(() => {
                this._isLoading = false ;
           })
    })
}

This block of code, once the access token is attempted to be refreshed by the KeycloakBearerInterceptor, effectively causes my spinner to hang, because the keycloak.js library sets up a setTimeout (which is considered a Macrotask) presumably so that knows to go and refresh the token.

[mauriciovigolo]

Hi @JohnSurina, I don't think it would be something to add to the current keycloak-bearer.interceptor, since it seems to be something very specific to your use case. I would suggest creating a separate interceptor in your project and to reference this interceptor instead of the one bundled in the library. There is no requirement to use the bearer interceptor, so you could create your own.

If more people find this useful we could also create a new interceptor implementation (new file added, not changing the existing one) in the future and give more options for the library users.

[mauriciovigolo]

I will close the issue, but feel free to answer or re-open the issue if desired.

[JohnSurina]

Thanks for the quick response, we've actually already got an interceptor that's handling updating when token are expired and I didn't see that I could disable the builtin interceptor via the service initializer until you mentioned I didn't need to use it. I'm doing a few tests right now to make sure I'm square (and I'm just peeking through the codebase to make sure there aren't any interceptor-like code that may run that I can't disable via configurations). I have a good feeling this'll work, and I'll report back my findings promptly.

[JohnSurina]

Okay, so including shouldUpdateToken: () => false in my keycloak.init options object DID make my problem with zone go away, so I'm happy to report that back.

That said, and I know you haven't had any other issues crop up with this flavor of question, I'll offer to make a pr to add a configuration "runOutsideAngular" that when true, all calls to keycloak.js will be wrapped in a "runOutsideAngular", and any zone-interfering function calls (like setTimeout, I can grep if you have any of the global function calls that zone.js considers macrotasks) will also be wrapped.

I understand if that's not something you'd like to see, since it would wrap several calls in the keycloak-service.ts file and in general just add lines to your library source, but I figure I'd offer in good faith in case it'd be a "nice to have" for you to consider pulling in at your leisure/discretion.

If it's something you'd appreciate, I can do it (won't take long), else I'll drop the subject unless I find another something that snags my team, which I don't anticipate.

Thanks again!

[JohnSurina]

The benefit of the proposed added configuration is that developers would be able to set it and not worry about any calls to the Keycloak service, or passive actions (like the interceptor) making any calls that'll add macrotasks to the angular zone. Again, I know it's niche, I just know that the NgZone API is an angular provided suite of functionality, and the hasPendingMacrotasks field is there for some reason, and the keycloak.js setToken method does leave a setTimeout open that'll mean that field is always true, unless it's run outside of the zone.