Using the timeout dialog and clicking the Keep alive button generates a JS error

[Jagordon]

I think specifically this happens when synchroniseTabs is undefined or false, and when initialising the app without sessionActivityService even though it doesn't need it.

My initialisation: if ($TimeoutDialog) { new TimeoutDialog($TimeoutDialog).init(); }

my njk template: {{ hmrcTimeoutDialog ({ timeout: timeoutDialog.timeout, countdown: timeoutDialog.countdown, keepAliveUrl: timeoutDialog.keepAliveUrl, signOutUrl: timeoutDialog.signOutUrl, timeoutUrl: timeoutDialog.timeoutUrl, title: t("timeout-modal:heading"), message: t("timeout-modal:message"), keepAliveButtonText: t("timeout-modal:keepAliveButtonText"), signOutButtonText: t("timeout-modal:deleteAnswersButtonText"), language: language, synchroniseTabs: false }) }}.

The Error:

Uncaught TypeError: Cannot read properties of undefined (reading 'logActivity') at HTMLButtonElement.keepAliveAndClose (timeout.js?20.8.0:1:6262)

[oscarduignan]

Hey @Jagordon - thank you very much this is really useful feedback - I think we mostly think about how tax services use the library, so we've maybe not implemented stuff as defensively as we could have - rather than a type error it would probably have been better to have a "session activity service required" error

the config added alongside this only controls whether the timeout dialog on the page it's loaded will reset itself if it receives activity broadcast from other tabs that are open

the timeout dialog always tries to broadcast session activity - regardless of that setting - so a sessionActivityService is always required

you could copy how we init the session activity service from our all.js file and not pass the BrowserBroadcastChannel if you wanted it to actually do nothing

the reason we added this is that we see that hmrc services often have timeout urls that will clear the current session, and the architecture of most tax services is that they share a frontend session (even across service / team boundaries)

so someone opening a page in the background to check something and then not revisiting it until after the timeout (but continuing to use another tax service, and keeping their session alive) could cause them to be signed out unexpectedly when the background tab was redirected to the timeout url if it was destructive

if your service timeout page isn't destructive to the session, and visiting it doesn't keep the session alive, then I can see how you might not need this - because background pages timing out wouldn't impact other pages - they would just display a timeout error

in the team we're going to discuss how we can be a bit more careful / clear when something like this changes, even if on the surface it seems like for most of our users it's an internal api change and not one we normally think of as external

[oscarduignan]

going to close this for now in the hope that's given you what you need to resolve the issue, but feel free to post any further questions if you have any and we'll keep an eye out (as an aside, we're going to setup a slack alert so we get notified about issues raised more quickly in the future, because it took us a bit of time to respond to this, apologies)

[Jagordon]

Hi Oscar, thanks for the reply. I understand the use case for it, but we have chosen not to use it (to be honest not 100% sure why, i inherited this project [maybe me going back and implementing the broadcast wouldn't hurt])

That said the broadcast is described as an optional feature that is not implemented as one. I would suggest that you either remove it as an optional feature and always add it into the timeout dialog, or stop the broadcast code firing if its optionally switched off.

[Jagordon]

Added an example MR

[oscarduignan]

Hey James, thank you for the PR, there's two parts to this though - the broadcasting of session activity is not an optional feature, it always happens - the optional feature is based on activity being broadcast from another tab, you can choose whether or not per-page you want to keep that pages timeout warning in sync with other tabs

So the API for instantiating the timeout dialog is now like this

  const sessionActivityService = new SessionActivityService(window.BroadcastChannel);

  sessionActivityService.logActivity();

  const $TimeoutDialog = document.querySelector('meta[name="hmrc-timeout-dialog"]');

  if ($TimeoutDialog) {
    new TimeoutDialog($TimeoutDialog, sessionActivityService).init();
  }

and if you didn't want to actually broadcast anything for some reason, then you could instantiate it like this

  const $TimeoutDialog = document.querySelector('meta[name="hmrc-timeout-dialog"]');

  if ($TimeoutDialog) {
    new TimeoutDialog($TimeoutDialog, new SessionActivityService()).init();
  }

so not pass a broadcast channel to the session activity service - where in the session activity service we check if there's a broadcast channel passed in and if there isn't then we do nothing

[Jagordon]

If its not optional you should refactor your code to not allow it to be optional.

In the interim please consider my PR to fix the existing bug.