fix: flush coverage data on SIGTERM/SIGINT#32362
Open
bartlomieju wants to merge 6 commits intodenoland:mainfrom
Open
fix: flush coverage data on SIGTERM/SIGINT#32362bartlomieju wants to merge 6 commits intodenoland:mainfrom
bartlomieju wants to merge 6 commits intodenoland:mainfrom
Conversation
Initial approach: intercept SIGTERM in the event loop when coverage collection is enabled, break out of the loop, and run the normal cleanup path (including stop_collecting). This is a WIP — needs to be reworked to use the deno_signals::before_exit pattern (like deno_telemetry does) instead of intercepting signals in the event loop. The challenge is that CoverageCollector uses a LocalInspectorSession which isn't Send, so it can't be called directly from the before_exit callback thread. Includes try_play/ directory with a Playwright + webServer reproduction case demonstrating the bug. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
When a Deno process with DENO_COVERAGE_DIR set receives SIGTERM (e.g. from Playwright's webServer teardown), coverage data is now flushed before exit. Previously the process would terminate without writing coverage files. Added sigterm_stream() and raise_sigterm_default() helpers to deno_signals crate. The worker event loop intercepts SIGTERM when coverage is enabled, breaks out to run the normal cleanup path (which flushes coverage via V8 inspector), then re-raises SIGTERM with the default handler for correct exit status. Unix-only for now. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Extend the sigterm_flush spec test to also verify that coverage data from web workers is flushed when the main process receives SIGTERM. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace the unix-only SIGTERM interception with cross-platform termination signal handling. Add `termination_signal_stream()` and `raise_default_signal()` helpers to deno_signals crate, removing platform-specific code from worker.rs. On unix: intercepts both SIGTERM and SIGINT On Windows: intercepts SIGINT (SIGTERM is not catchable on Windows) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The try_play/ directory was used for local debugging and shouldn't be part of the PR. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Member
Author
|
Needs a bit more work to flush coverage data periodically to guard against |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes coverage data loss when a Deno process with
DENO_COVERAGE_DIRisterminated via SIGTERM or SIGINT (e.g. by Playwright's
webServerteardown,Docker, systemd, Ctrl+C).
Currently,
CoverageCollector::stop_collecting()only runs via the normal exitpath in
CliMainWorker::run(). If the process receives a termination signal,the event loop exits immediately and coverage data is never written to disk.
Approach
When coverage collection is enabled, the event loop in
cli/worker.rsusestokio::select!to race the normal event loop future against a terminationsignal stream. On receiving SIGTERM or SIGINT:
unload/processExitevent dispatch (process is being killed)stop_collecting()to flush coverage via V8 inspectorwith the correct signal exit code
Changes
ext/signals/lib.rs— Added cross-platform helpers todeno_signals:SignalStreamWithKind— async stream that reports which signal was caughttermination_signal_stream()— listens for SIGTERM+SIGINT on unix, SIGINTon Windows
raise_default_signal(signo)— re-raises a signal with the default handlercli/worker.rs— ModifiedCliMainWorker::run()to intercept terminationsignals when coverage is enabled, flush coverage data, then re-raise the signal
tests/specs/coverage/sigterm_flush/— Spec tests verifying coverage iswritten on SIGTERM for both main worker and web worker scenarios
Limitations
TerminateProcess(uncatchable by design).A follow-up PR will add periodic coverage flushing to handle force-kill
scenarios.
Reproduction
The original issue was discovered with Playwright's
webServerconfig:Playwright sends SIGTERM (with
gracefulShutdown) or SIGKILL (default) whentearing down the dev server. With
DENO_COVERAGE_DIRset inwebServer.env,coverage was never written.
Test plan
cargo test sigterm_flush— spawns a Deno server withDENO_COVERAGE_DIR, sends SIGTERM, verifies coverage files are writtenand worker coverage are flushed (2 files)
deno run --allow-net server.tswithDENO_COVERAGE_DIRset,send SIGTERM, verify
.jsonfiles appear in coverage dir🤖 Generated with Claude Code