gildas-lormeau / single-file-cli

CLI tool for saving a faithful copy of a complete web page in a single HTML file (based on SingleFile)
GNU Affero General Public License v3.0
602 stars 63 forks source link

Handle SIGTERM and SIGINT #79

Closed sissbruecker closed 5 months ago

sissbruecker commented 5 months ago

Currently single-file-cli does not handle SIGTERM and SIGINT. SIGINT occurs when terminating the process though the CLI when pressing Cmd/Ctrl+C. SIGTERM occurs when running single-file as a child process in another app, and sending the SIGTERM signal to that process so that it can properly terminate itself.

The current behavior is:

This adds listeners for both SIGINT and SIGTERM and runs closeBrowser in them, so that both Chromium is terminated and the profile folder is removed.

For testing SIGINT, just start single-file from the CLI and press Cmd/Ctrl+C. To observe whether the profile was deleted, something can be printed to the console in closeBrowser.

For testing SIGTERM I was using these scripts:

Deno ```js const command = new Deno.Command(Deno.execPath(), { args: [ "run", "--allow-read", "--allow-write", "--allow-run", "--allow-net", "single-file", "https://github.com/wagoodman/dive", "test.html", ], stdin: "piped", stdout: "piped", stderr: "piped", }); const subprocess = await command.spawn(); subprocess.stdout.pipeTo( Deno.openSync("output", { write: true, create: true }).writable, ); subprocess.stderr.pipeTo( Deno.openSync("output_err", { write: true, create: true }).writable, ); setTimeout(() => { subprocess.kill("SIGTERM"); }, 1000); ``` Run as `deno run --allow-read --allow-write --allow-run test-deno.js`
Node ```js import child_process from 'child_process'; const child = child_process.spawn('node', ['single-file-node.js', 'https://github.com/wagoodman/dive', 'test.html']); child.stdout.pipe(process.stdout); child.stderr.pipe(process.stderr); setTimeout(() => { child.kill('SIGTERM') }, 1000); ``` Run as: `node test-node.js`
gildas-lormeau commented 5 months ago

I need to do some tests but isn't this issue specific to Node.js?

EDIT: actually, Ctrl-C works fine on macOS on my end with Deno and Node.js. In both cases, Chrome is stopped. Note that the call to child.ref() in browser.js is supposed to handle all these cases (except the test code === 130) if I am not wrong.

EDIT #2: Moreover, on Node.js detached is net set to true when calling childProcess.spawn() in deno-polyfill.js.

I'll try your scripts

gildas-lormeau commented 5 months ago

Ok, I am wrong about ref(). It's the other way around actually so I'm OK with the PR.

sissbruecker commented 5 months ago

Ctrl-C does stop Chrome, but it does not run closeBrowser. So I guess it's related to how the process is created in the shell, and Ctrl-C kills all spawned child processes as well.

sissbruecker commented 5 months ago

Thanks!

gildas-lormeau commented 5 months ago

You're welcome. I would like to thank you again for your contribution and the test examples. I've just published the new version 2.0.28 with your fix.