Node not saving coverage when process is terminated using SIGINT/SIGTERM

[aalexgabi]

• Version: v14.10.1

What steps will reproduce the bug?

When running this code

function run() {
  if (1) {
    console.log(1);
  } else {
    console.log(2);
  }
}

run();

setTimeout(process.exit, 3000)

If I stop the process using SIGINT/SIGTERM before the timeout kicks in, coverage is not saved.

% NODE_V8_COVERAGE=/tmp/cov node /tmp/t.js
1
^C
% ls /tmp/cov
ls: cannot access '/tmp/cov': No such file or directory
% 

What is the expected behavior?

Node coverage should work in the same way as other coverage tools, saving the coverage before terminating the process even if no SIGINT handler is set.

What do you see instead?

No coverage is saved.

Additional information

I know that adding a SIGINT/SIGTERM handler can be used as a workaround but this is not how most applications terminate and has no added value.

process.on('SIGINT', () => process.exit())
process.on('SIGTERM', () => process.exit())

[aalexgabi]

For reference the issue reported by me in c8 repository: bcoe/c8#189

[bnoordhuis]

This is basically a duplicate of #27406 (which itself is a partial duplicate of #24937) so I'm going to close it as such. Thanks anyway for the report.

[aalexgabi]

@bnoordhuis I don't see how it's a duplicate at all. Here I'm talking about saving coverage data not cpu prof.

If I attach an event listener to node process, coverage is dumped so there is no technical limitation to implement this behavior by default.

A process can receive a signal multiple times and it can delay processing of a signal so there is plenty of time to write the files to the disk.

[joyeecheung]

I guess this isn't really a duplicate - even after we fix the same issue with --cpu-prof we would still need to add extra code to handle coverage.

[mhdawson]

@bcoe I know you are up to speed on the coverage front.

[bcoe]

I believe this used to work ™️. When/if we figure out an approach for --cpu-prof, let's make sure we apply it to coverage too.

Most importantly, let's make sure we add a regression test (could have sworn we had won, funny enough).

[joyeecheung]

I took a look and I think we'd need to just create a different watchdog for the profilers. I'll be refactoring the entire profiler connection class for #33807 and I'll look into how to throw a watchdog into the class

[Dzenly]

Hi! Could someone explain me (or give me a link) how these signal handlers affect v8 coverage behaviour?
I've read https://v8.dev/blog/javascript-code-coverage, but did not find any signals mention.

I.e. I would understand if there was recommendation to call v8.takeCoverage() from SIGTERM handler. But how does this work with only process.exit() in SIGTERM handler ?

[bnoordhuis]

@Dzenly Something like process.on("SIGTERM", v8.stopCoverage) is allowed and should work. The caveats around signal handling are about handling them in C++, not JS. The one JS caveat is that process.on("SIGTERM", ...) won't work when the script is stuck in a busy loop.