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

Unreachable URL error terminates process without closing browser when using Node #81

Closed sissbruecker closed 5 months ago

sissbruecker commented 5 months ago

cdp-client.js throws an error when a URL can not be reached: https://github.com/gildas-lormeau/single-file-cli/blob/2b1ddaeb5219dfd19d1a33507fd6d5a017e9ed75/lib/cdp-client.js#L188

The error handler in run in single-file-launcher.js doesn't seem to be effective in this case because the error is thrown from a function that is registered as an event listener, which puts it outside of the scope of execution of single-file when the event listener is called: https://github.com/gildas-lormeau/single-file-cli/blob/2b1ddaeb5219dfd19d1a33507fd6d5a017e9ed75/lib/cdp-client.js#L170

That results in an unhandled exception in the Node process, which results in Chromium not being stopped when running single-file as a subprocess from another app:

import child_process from 'child_process';

const child = child_process.spawn('node', ['single-file-node.js', 'https://makers.snips.ai/', 'test.html']);
child.stdout.pipe(process.stdout);
child.stderr.pipe(process.stderr);

Only seems to happen with Node this time. In Deno it gives an unhandled error too, but the browser process is stopped for some reason. A workaround for Node would be to add global error handlers and close the browser there:

process.once("uncaughtException", async (error) => {
    console.error(error.message || error); // eslint-disable-line no-console
    await closeBrowser();
    exit(-1);
});

I'd also suggest to run closeBrowser from the error handler in run.

gildas-lormeau commented 5 months ago

Thank you very much for the detailed description, I've just fixed the bug. Now the exception is handled properly.