Skip to content

fix: properly await worker teardown in integration tests to prevent open handles#481

Merged
cameri merged 12 commits intomainfrom
fix/integration-test-hanging-open-handle
Apr 18, 2026
Merged

fix: properly await worker teardown in integration tests to prevent open handles#481
cameri merged 12 commits intomainfrom
fix/integration-test-hanging-open-handle

Conversation

@phoenix-server
Copy link
Copy Markdown
Collaborator

Summary

  • AfterAll was calling worker.close(callback) without awaiting it, so Cucumber resolved the hook immediately while the HTTP server, Redis client, and DB connections remained open
  • Node.js would then hang indefinitely waiting for those handles to close, causing integration tests to never exit
  • Fixed by wrapping worker.close() in a Promise so AfterAll properly waits for all connections to be torn down before the process exits

Root cause

worker.close() is callback-based (like http.Server.close()). Inside an async function, calling it without wrapping in a Promise means the function returns immediately — the callback never blocks the hook from completing.

Test plan

  • Run npm run docker:test:integration and confirm tests exit cleanly without hanging
  • Confirm no open handle warnings from Node.js after test run

🤖 Generated with Claude Code

…pen handles

AfterAll was calling worker.close() with a callback but not awaiting it,
so Cucumber resolved the hook immediately leaving the HTTP server, Redis,
and DB connections open. Node.js would then hang indefinitely waiting for
those handles to close.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@coveralls
Copy link
Copy Markdown
Collaborator

coveralls commented Apr 18, 2026

Coverage Report for CI Build 24596779791

Coverage decreased (-0.3%) to 48.812%

Details

  • Coverage decreased (-0.3%) from the base build.
  • Patch coverage: 2 uncovered changes across 1 file (6 of 8 lines covered, 75.0%).
  • 8 coverage regressions across 2 files.

Uncovered Changes

File Changed Covered %
src/cache/client.ts 4 2 50.0%

Coverage Regressions

8 previously-covered lines in 2 files lost coverage.

File Lines Losing Coverage Coverage
src/cache/client.ts 6 40.91%
src/app/worker.ts 2 55.26%

Coverage Stats

Coverage Status
Relevant Lines: 2633
Covered Lines: 1338
Line Coverage: 50.82%
Relevant Branches: 1071
Covered Branches: 470
Branch Coverage: 43.88%
Branches in Coverage %: Yes
Coverage Strength: 9.53 hits per line

💛 - Coveralls

github-manager and others added 11 commits April 17, 2026 23:19
Redis client may not be connected when rate limits are disabled in tests,
causing disconnect() to throw ClientClosedError. Guard with isOpen check.
Also raise AfterAll timeout to 30s to allow server and DB to shut down.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
http.Server.close() waits for keep-alive connections to drain naturally,
which can prevent the close callback from ever firing. closeAllConnections()
forces them closed immediately so the server shuts down reliably in CI.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… tests

ReplaySubject(2, 1000) schedules recurring setTimeout calls to evict stale
buffered events. These timers outlive the test run and keep Node.js alive
after Cucumber finishes, preventing clean process exit on CI.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The slidingWindowRateLimiterFactory creates a RedisAdapter singleton that
calls client.connect() on first WebSocket connection and has no teardown
path. --force-exit ensures the process exits cleanly after tests complete
rather than hanging on the open Redis socket.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
--force-exit calls process.exit before nyc flushes coverage, causing
integration coverage to drop. Instead, call process.exit(0) at the end
of AfterAll after all cleanup awaits complete, giving nyc a clean exit.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…owTime changes

These were exploratory fixes for the hang; the actual fix was process.exit(0)
in AfterAll after cleanup completes.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
process.exit(0) in AfterAll kills the process before Cucumber prints
its summary. An unref'd 2s timeout lets Cucumber finish output, then
force-exits if the Redis singleton is still keeping the process alive.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add closeCacheClient() to the cache module and call it inside AppWorker.close()
after the adapter shuts down. This gives the Redis singleton a proper teardown
path instead of relying on test-only workarounds.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Process still hangs after worker.close() + DB destroy. Watchdog fires
after 2s, logs all active handles/requests via process._getActiveHandles()
so we can identify the culprit, then force-exits.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…loud

The --publish flag was uploading test results to reports.cucumber.io via
a TLS connection to Cloudflare R2, which kept the process alive after
tests completed. Reports are already saved locally as HTML and JSON.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@cameri cameri merged commit b3b6176 into main Apr 18, 2026
10 checks passed
@cameri cameri deleted the fix/integration-test-hanging-open-handle branch April 18, 2026 04:25
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.

3 participants