Skip to content

fix(test): close leaked fetch contexts and OAuth callback timers to stop suite hang - #2797

Open
tripodsan wants to merge 1 commit into
mainfrom
fix/test-suite-hang-open-handles
Open

tripodsan wants to merge 1 commit into
mainfrom
fix/test-suite-hang-open-handles

Conversation

@tripodsan

Copy link
Copy Markdown
Contributor

Please ensure your pull request adheres to the following guidelines:

  • make sure to link the related issues in this description
  • when merging / squashing, make sure the fixed issue references are visible in the commits, for easy compilation of release notes

Related Issues

N/A — found while investigating a reported hang at the end of npm test.

Summary

npm test (c8 mocha) ran all tests successfully but the process kept running for several extra minutes after mocha reported the final "passing" summary, before finally exiting. Root cause: multiple leaked async resources kept the Node event loop alive.

  1. Leaked @adobe/fetch keep-alive contexts

    • getFetch() in src/fetch-utils.js lazily creates and caches a keepAlive() fetch context (an open-socket connection pool). Production code paths always call resetContext() on shutdown, but no test ever did, so the shared context (used across test/server.test.js, test/up-cmd.test.js, test/utils.js) stayed open for the whole run.
    • An ad-hoc h1NoCache() context created per-request inside the http-proxy test in test/server.test.js was never reset.
    • A module-level h1NoCache() context in test/import-cmd.test.js was never reset.
  2. Leaked OAuth callback timer

    • waitForToken() in src/content/da-auth.js starts a real 5-minute setTimeout safety timer that is only cleared once a /token callback request arrives on its local HTTP server.
    • Several tests in test/content/da-auth.test.js mocked that HTTP server as a no-op (or raced the real flow against an uncancelled 100ms timeout via Promise.race), so the callback never fired and the 5-minute timer kept running in the background — exactly matching the observed multi-minute delay before process exit.

Fix

  • test/setup-env.js: added a global mochaHooks.afterAll hook that calls resetContext() to close the shared getFetch() context after the whole suite runs.
  • test/server.test.js: reuse a single h1NoCache() context for the http-proxy test and reset() it in the finally block.
  • test/import-cmd.test.js: reset() the module-level fetch context in an after() hook.
  • test/content/da-auth.test.js: mock the HTTP callback server to simulate an immediate /token success response so waitForToken() resolves and clears its timer deterministically, instead of leaking it for up to 5 minutes. Removed the now-unnecessary Promise.race/timeout workaround.

Verification

  • npx mocha and npm test (c8 mocha) both now exit on their own immediately after reporting results (548 passing), instead of hanging for several extra minutes.
  • Confirmed via process._getActiveHandles() / timer-tracing instrumentation inside the actual mocha worker process that all previously-leaked sockets/timers are now properly closed/cleared.
  • npx eslint passes on all changed files.

Thanks for contributing!

…top suite hang

npm test / c8 mocha finished all tests but hung for several extra minutes
before the process actually exited, because:

- getFetch() in src/fetch-utils.js caches a keep-alive @adobe/fetch context
  that is never reset() in tests, leaving open sockets that keep the event
  loop alive.
- An ad-hoc h1NoCache() fetch context in the http-proxy test and a
  module-level one in import-cmd.test.js were never reset either.
- Several da-auth.test.js tests exercised the real OAuth login flow
  (waitForToken in src/content/da-auth.js), which starts a real 5-minute
  setTimeout safety timer that is only cleared once a /token callback
  request arrives. The mocked HTTP server never simulated that callback (or
  raced it against an uncancelled 100ms timeout), so the timer kept the
  process alive for up to 5 minutes after mocha reported all tests passing.

Fixes:
- test/setup-env.js: add a global mochaHooks.afterAll that calls
  resetContext() to close the shared getFetch() keep-alive context.
- test/server.test.js: reuse a single h1NoCache() context for the proxy test
  and reset() it in the finally block.
- test/import-cmd.test.js: reset() the module-level fetch context after all
  tests in the file.
- test/content/da-auth.test.js: mock the HTTP callback server to simulate an
  immediate /token success so waitForToken() resolves and clears its timer
  deterministically, instead of leaking it.

Verified: npx mocha and npm test now both exit on their own right after
reporting results (548 passing), instead of hanging for minutes afterwards.

Co-authored-by: Copilot <[email protected]>
@tripodsan
tripodsan requested a review from kptdobe September 29, 2026 15:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants