Proactive token refresh blocks graceful Node.js exits

[rubensworks]

Search terms you've used

refresh, token, exit, hanging

Impacted package

Which packages do you think might be impacted by the bug ?

• [ ] solid-client-authn-browser
• [x] solid-client-authn-node
• [ ] solid-client-authn-core
• [ ] oidc-client-ext
• [ ] Other (please specify): ...

Bug description

When using this lib in a Node.js application, the Node.js will not gracefully exit anymore after logging in. This appears to be caused by the timeouts that are set to handle token refreshing, which obviously makes sense.

I was wondering if it would be possible to work around this, by for example

1. disabling token refreshing, or
2. forcefully clearing the latest timeout handler.

To Reproduce

I've created a small repo with a bin script that demonstrates the problem. After logging in, the Node process will hang.

Using a packages such as why-is-node-running, it can be observed that it is hanging because of the mentioned timeout handler.

Environment

Please run

npx envinfo --system --npmPackages --binaries --npmGlobalPackages --browsers

in your project folder and paste the output here:

  System:
    OS: macOS 10.15.4
    CPU: (8) x64 Intel(R) Core(TM) i7-1068NG7 CPU @ 2.30GHz
    Memory: 44.97 MB / 16.00 GB
    Shell: 3.2.57 - /bin/bash
  Binaries:
    Node: 16.2.0 - /usr/local/bin/node
    Yarn: 1.22.4 - /usr/local/bin/yarn
    npm: 7.13.0 - /usr/local/bin/npm
  Browsers:
    Chrome: 92.0.4515.131
    Firefox: 90.0.2
    Safari: 13.1
  npmPackages:
    @inrupt/solid-client-authn-node: ^1.11.0 => 1.11.0
    @rubensworks/eslint-config: ^1.0.0 => 1.0.1
    @types/jest: ^26.0.0 => 26.0.24
    @typescript-eslint/eslint-plugin: ^4.2.0 => 4.29.2
    @typescript-eslint/parser: ^4.1.1 => 4.29.2
    coveralls: ^3.0.0 => 3.1.1
    eslint: ^7.9.0 => 7.32.0
    eslint-config-es: 3.23.0 => 3.23.0
    eslint-import-resolver-typescript: ^2.3.0 => 2.4.0
    eslint-plugin-import: ^2.22.0 => 2.24.0
    eslint-plugin-jest: ^24.0.2 => 24.4.0
    eslint-plugin-tsdoc: ^0.2.7 => 0.2.14
    eslint-plugin-unused-imports: ^1.0.0 => 1.1.4
    jest: ^26.0.0 => 26.6.3
    manual-git-changelog: ^1.0.0 => 1.0.1
    open: ^8.2.1 => 8.2.1
    pre-commit: ^1.2.2 => 1.2.2
    ts-jest: ^26.0.0 => 26.5.6
    typescript: ^4.3.5 => 4.3.5
  npmGlobalPackages:
    @comunica/actor-init-sparql-file: 1.14.0
    @ldflex/comunica: 3.4.2
    chrome-headless-render-pdf: 1.9.0
    clinic: 8.0.1
    componentsjs-generator: 2.6.0
    generator-comunica: 1.0.0
    http-server: 0.12.3
    ldbc-snb-enhancer: 1.0.2
    n: 6.5.1
    npm-cli-adduser: 1.1.4
    npm: 7.13.0
    rdf-dataset-fragmenter: 1.2.0
    rdf-dereference: 1.5.0
    renovate: 23.20.2
    verdaccio: 3.11.4
    why-is-node-running: 2.2.0
    yo: 3.1.1

Additional information

[NSeydoux]

Thanks for reporting the issue @rubensworks ! In your case, which would make more sense: being able to disable the background refreshing altogether, or being able to clear the latest one, probably on user logout ? I think the former would be easy, but if the latter would be more useful to you, as long as the issue is not too disruptive I can investigate a bit more and come up with a cleaner solution.

[rubensworks]

which would make more sense: being able to disable the background refreshing altogether, or being able to clear the latest one, probably on user logout ?

As the former is the easiest solution, I guess that should be fine for my use case for now.

However, the manual clearing of timeouts on logout would also be useful in the long term. For example, I'm using this for query execution. So if there are long-running queries, there may be still a need for background refreshing. Once the query terminates, a manual clear/logout could then be done.

[NSeydoux]

Well, it turns out that clearing things up on logout was easier than preventing the background refresh in the first place, because it demanded no API change :). Do let me know if preventing the background refresh altogether is still something that you'd require. You can test if this fixes your issue using @inrupt/solid-client-authn-node@fixnodeproactive-refresh. I won't close the issue until the fix is properly released and you've confirmed that your issue is resolved :)

[rubensworks]

Thanks @NSeydoux! Will try it out soon.

[AJamesPhillips]

Very minor error @rubensworks but you might like to rename the title from "exists" to "exits".

[rubensworks]

Oh, oops, you're right 😅

[rubensworks]

@NSeydoux The fix works like a charm, thanks for the quick fix!

[NSeydoux]

I'm glad it works !