Skip to content

Commit

Permalink
fix: remove uncaught exception handler (#154)
Browse files Browse the repository at this point in the history
There are no circumstances under which it is safe to ignore uncaught
exceptions, since the application is left in an unconsistent state.

The node documentation is unequivocal about this:

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

Any code that can throw an exception should be wrapped in a `try/catch`
or there should be some other method of handling the error.
  • Loading branch information
achingbrain authored Jul 9, 2024
1 parent 87deb92 commit 43af7c5
Showing 2 changed files with 1 addition and 39 deletions.
25 changes: 0 additions & 25 deletions src/constants.ts
Original file line number Diff line number Diff line change
@@ -80,28 +80,3 @@ export const GC_TIMEOUT_MS = 20000
* How long to wait for the healthcheck retrieval of an identity CID to complete
*/
export const HEALTHCHECK_TIMEOUT_MS = 1000

/**
* You can set `RECOVERABLE_ERRORS` to a comma delimited list of errors to recover from.
* If you want to recover from all errors, set `RECOVERABLE_ERRORS` to 'all'.
* If you want to recover from no errors, set `RECOVERABLE_ERRORS` to ''.
*/
export const RECOVERABLE_ERRORS = (() => {
if (process.env.RECOVERABLE_ERRORS === 'all') {
return 'all'
}
if (process.env.RECOVERABLE_ERRORS === '') {
return ''
}
return process.env.RECOVERABLE_ERRORS?.split(',') ?? 'all'
})()

export const ALLOW_UNHANDLED_ERROR_RECOVERY = (() => {
if (RECOVERABLE_ERRORS === 'all') {
return true
}
if (RECOVERABLE_ERRORS === '') {
return false
}
return RECOVERABLE_ERRORS.length > 0
})()
15 changes: 1 addition & 14 deletions src/index.ts
Original file line number Diff line number Diff line change
@@ -68,7 +68,6 @@
* | `ECHO_HEADERS` | A debug flag to indicate whether you want to output request and response headers | `false` |
* | `USE_DELEGATED_ROUTING` | Whether to use the delegated routing v1 API | `true` |
* | `DELEGATED_ROUTING_V1_HOST` | Hostname to use for delegated routing v1 | `https://delegated-ipfs.dev` |
* | `RECOVERABLE_ERRORS` | A comma delimited list of errors to recover from. These errors are checked in `uncaughtException` and `unhandledRejection` callbacks | `all` |
*
* <!--
* TODO: currently broken when used in docker, but they work when running locally (you can cache datastore and blockstore locally to speed things up if you want)
@@ -161,7 +160,7 @@ import cors from '@fastify/cors'
import { createVerifiedFetch } from '@helia/verified-fetch'
import fastify, { type FastifyInstance, type RouteOptions } from 'fastify'
import metricsPlugin from 'fastify-metrics'
import { HOST, HTTP_PORT, RPC_PORT, METRICS, ECHO_HEADERS, FASTIFY_DEBUG, RECOVERABLE_ERRORS, ALLOW_UNHANDLED_ERROR_RECOVERY } from './constants.js'
import { HOST, HTTP_PORT, RPC_PORT, METRICS, ECHO_HEADERS, FASTIFY_DEBUG } from './constants.js'
import { contentTypeParser } from './content-type-parser.js'
import { getCustomHelia } from './get-custom-helia.js'
import { httpGateway } from './helia-http-gateway.js'
@@ -232,18 +231,6 @@ async function closeGracefully (signal: number | string): Promise<void> {
process.once(signal, closeGracefully)
})

const uncaughtHandler = (error: any): void => {
log.error('Uncaught Exception:', error)
if (ALLOW_UNHANDLED_ERROR_RECOVERY && (RECOVERABLE_ERRORS === 'all' || RECOVERABLE_ERRORS.includes(error?.code) || RECOVERABLE_ERRORS.includes(error?.name))) {
log.trace('Ignoring error')
return
}
void closeGracefully('SIGTERM')
}

process.on('uncaughtException', uncaughtHandler)
process.on('unhandledRejection', uncaughtHandler)

interface ServerOptions {
metrics: boolean
}

0 comments on commit 43af7c5

Please sign in to comment.