authts
design defect: minimum `_expiringTimer` forced to five seconds
First let me say that I'm very impressed with the oidc-client-tc library. It's well-designed, flexible, modular, and extensible. I've recently pulled off some complicated functionality by extending several of the library's internal interfaces. Great job on the design.
Being so impressed with the library leaves me in dismay at the following code I found, which is unexpected, obscured, problematic, and unconfigurable.
In AccessTokenEvents the _expiringTimer is supposed to be set to wake up just before the token expires—specifically accessTokenExpiringNotificationTimeInSeconds seconds before the token expires. This value defaults to 60 seconds. (See UserManagerSettings.ts.) All this is reasonable.
Unfortunately that's not exactly what happens, because of this hidden code inside Timer:
// we're using a fairly short timer and then checking the expiration in the
// callback to handle scenarios where the browser device sleeps, and then
// the timers end up getting delayed.
const timerDurationInSeconds = Math.min(durationInSeconds, 5);
I'm in a particular situation where I don't want a timer going off every five seconds, even if the callback double-checks the expiration time.
This functionality is completely hidden, counterintuitive, and in contradiction to the log messages. All the debug log messages talk about how the timer will go off in durationInSeconds (e.g. logger.debug("registering expiring timer, raising in", expiring, "seconds")). There is no way for anyone to know that the debug messages are not truthful, and that really the timer is raising in five seconds, unless they were to look at the code.
Moreover there is absolutely no way to easily configure this. The value 5 is hard-coded!!
I understand that someone at some time had some issue with something going to sleep. But hard-coding the expiration timer at a five-second timer minimum is not the best approach, and stands in contrast to the well-designed configurability of the rest of the library. If you simply wanted something to keep from going to sleep, it would be most appropriate for a separate timer to be used rather than to overload the semantics of the expiring token timer, for example. And whatever is done, the five-second interval needs to be configurable.
Unfortunately that's not exactly what happens, because of this hidden code inside Timer:
Browsers are nowadays going to freeze/sleep tabs as soon as possible to avoid power consumption and maybe even memory consumption . This means this particular code part was done for newer browser and its not old cruft.
Moreover there is absolutely no way to easily configure this. The value 5 is hard-coded!!
But i agree with you that it should be made configurable. Please feel free to provide a merge request to allow either changing the hard coding 5s by a new setting (default is 5s) or something better.
Browsers are nowadays going to freeze/sleep tabs as soon as possible to avoid power consumption and maybe even memory consumption . This means this particular code part was done for newer browser and its not old cruft.
But it makes the assumption that it's running in a browser tab! Maybe I want to run it in a browser extension background service worker.
In any case it shouldn't have some secret hard-coded force hidden away that is directly contradictory to the documentation, the API, and even the logging messages.
But it makes the assumption that it's running in a browser tab! Maybe I want to run it in a browser extension background service worker.
Ah now i understand your problem. This library was initially not designed for a browser extension background service worker. I am only using it within browser tabs. But i am open to accept merge request to improve the situation for other scenarios.
I have not much experience with browser extensions, but probably the whole slicing const timerDurationInSeconds = Math.min(durationInSeconds, 5); make no sense for you. Thus feel free to provide an option to disable that slicing if that makes it work on your side....
Ah now i understand your problem.
That is not "the problem". That was one example of the multitude of side effects that can happen from hard-coding a secret override to quickly work around a problem instead of taking the time to integrate it correctly, with configuration settings, updated log messages, and an API that coincided with what actually happens. The "problem" here is that we have a secret, hard-coded override hidden in the code that overrides the express intent of the user they configured via the API.
This was also discussed in https://github.com/authts/oidc-client-ts/issues/644 but it went stale...
