Importing this library causes uncaught errors to be printed without stack traces

[Yuval-Peled]

Bug description
Importing this library causes uncaught errors to be printed without stack traces. Note - it is enough to import the library, you don't even have to call any of its functions.

To Reproduce
Steps to reproduce the behavior:

1. Create a file with the following contents: const httpsLocalhost = require('https-localhost'); throw new Error();
2. Run the file. The output is:

Unexpected error undefined:

Error

Process finished with exit code 1

Expected behavior
The Error should be logged normally with node.js default behavior, which looks like this:

throw new Error();
^

Error
    at Object.<anonymous> (/Users/myuser/scripts/test.js:1:7)
    at Module._compile (node:internal/modules/cjs/loader:1101:14)
    at Object.Module._extensions..js (node:internal/modules/cjs/loader:1153:10)
    at Module.load (node:internal/modules/cjs/loader:981:32)
    at Function.Module._load (node:internal/modules/cjs/loader:822:12)
    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:81:12)
    at node:internal/main/run_main_module:17:47

Process finished with exit code 1

Specifically, the print should contain a stack trace, which is critical to debug applications.

System information (please complete the following information):

• OS: MacOS Big Sur 11.5.2
• package version 4.7.0
• node version 16.13.0

Additional context
This is caused because https-localhost has a global error handler implemented here:

https-localhost/index.js

Lines 101 to 118 in b485509

	process.on("uncaughtException", function(err) {
	switch (err.errno) {
	case "EACCES":
	console.error(
	"EACCES: run as administrator to use the default ports 443 and 80. " +
	"You can also change port with: `PORT=4433 serve ~/myproj`.")
	break
	case "EADDRINUSE":
	console.error("EADDRINUSE: another service on your machine is using " +
	"the current port.\nStop it or change port with:" +
	"`PORT=4433 serve ~/myproj`.")
	break
	default:
	console.error("Unexpected error " + err.errno + ":\n\n" + err)
	break
	}
	process.exit(1)
	})

In my use case, we only use this library during development, according to some environment variable. It looks something like this:

import httpsLocalhost from 'https-localhost';
if (process.env['RUN_HTTPS_LOCALHOST')) { .... }

This means that even though we don't run https-localhost in production, we do import it. This causes the global error handler to run and takes over my app's behavior regarding Errors, making it much harder to debug uncaught errors since they don't have a stack trace because of this issue.
We solved this by using a dynamic import, but the root cause was very hard to find. This is our soluton:

if (process.env['RUN_HTTPS_LOCALHOST')) { 
  const httpsLocalhost = await import('https-localhost').default;
  ...
}

My suggestion:
An imported library should not implement a global error handler. I imagine this is implemented for the CLI, so that uncaught errors will cause the CLI to immediately bail. The error handler logic should only be implemented if this is run through the CLI.

[github-actions]

Marked as stale for no activity in the past 60 days. In 7 days it will be closed unless new activity.

[daquinoaldo]

@Yuval-Peled, could you open a pull request with proposed changes? I like your analysis and I’ll be glad to evaluate a proposal.

[Yuval-Peled]

@daquinoaldo
Yes, I'll look into that.

[github-actions]

Marked as stale for no activity in the past 60 days. In 7 days it will be closed unless new activity.