ipfs / helia-http-gateway

Run Helia NodeJS in a Docker Container.
Other
8 stars 6 forks source link

fix: uncaughtException needs more best practices #112

Open SgtPooki opened 3 months ago

SgtPooki commented 3 months ago
### Tasks
- [ ] default `ALLOW_UNHANDLED_ERROR_RECOVERY` to false
- [ ] Add warning in readme
- [ ] Add default known-recoverable uncaughtExceptions
- [ ] Add listener to req & resp for ERR_STREAM_PREMATURE_CLOSE

https://github.com/ipfs/helia-http-gateway/blob/6f10038693de6001d332d2e92ff666f8f20b9c1c/src/index.ts#L273-L280

related:

cc @achingbrain

achingbrain commented 3 months ago

Add default known-recoverable uncaughtExceptions

There are no recoverable uncaught exceptions. This is clearly documented:

The event should not be used as an equivalent to On Error Resume Next. Unhandled exceptions inherently mean that an application is in an undefined state. Attempting to resume application code without properly recovering from the exception can cause additional unforeseen and unpredictable issues.

The correct use of 'uncaughtException' is to perform synchronous cleanup of allocated resources (e.g. file descriptors, handles, etc) before shutting down the process. It is not safe to resume normal operation after 'uncaughtException'.

https://nodejs.org/api/process.html#warning-using-uncaughtexception-correctly

Note:

It is not safe to resume normal operation after 'uncaughtException'.

yet this is exactly what #102 does.