feat(appkit): add global uncaughtException/unhandledRejection handlers#448
Closed
MarioCadenas wants to merge 3 commits into
Closed
feat(appkit): add global uncaughtException/unhandledRejection handlers#448MarioCadenas wants to merge 3 commits into
MarioCadenas wants to merge 3 commits into
Conversation
Unhandled async errors previously crashed the Node process silently in production. Register process-level uncaughtException/unhandledRejection handlers in the server plugin, in the same place SIGTERM/SIGINT are wired. uncaughtException logs and runs the existing graceful-shutdown path then exits non-zero; unhandledRejection logs at error level. A module-wide guard registers the handlers once per process so repeated start() calls don't stack listeners. Co-authored-by: Isaac Signed-off-by: MarioCadenas <MarioCadenas@users.noreply.github.com>
…ervers set Replaces the module-level boolean guard with a static startedServers Set whose emptiness is the install-once latch: registration stays lazy (importing the module attaches nothing) and needs no separate mutable flag. Handlers now shut down every started server rather than only the first instance to start (the boolean closed over the first instance), and the per-instance method becomes a static installer. Co-authored-by: Isaac Signed-off-by: MarioCadenas <MarioCadenas@users.noreply.github.com>
Reverts the started-servers Set over-engineering. A single server runs per process, so a plain once-flag is sufficient; it only exists to stop repeated start() in tests from stacking listeners. Also trims verbose comments. Co-authored-by: Isaac Signed-off-by: MarioCadenas <MarioCadenas@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The server bootstrap registers
SIGTERM/SIGINThandlers but had no globaluncaughtException/unhandledRejectionhandlers. An unhandled async errorwould crash the Node process silently in production — no structured log, no
graceful drain of in-flight work.
What changed
In
packages/appkit/src/plugins/server/index.ts, the signal handlers are nowregistered via a new
_registerProcessHandlers()method (called fromstart()in the same spot SIGTERM/SIGINT were wired), which also adds:
uncaughtException— logs the error (withorigin) via the existingserverlogger, then runs the same_gracefulShutdown()path the signalhandlers use (drain plugins, close server) and exits non-zero.
_gracefulShutdownnow takes an
exitCode(default0) so the success path stays0while afatal exception exits
1.unhandledRejection— logs at error level for visibility, without forcingan exit, matching how the codebase isolates non-fatal errors (Node best practice).
A module-level
processHandlersRegisteredguard ensures handlers are registeredexactly once per process, regardless of how many server instances start or how
many times
start()runs. This also removes the pre-existingMaxListenersExceededWarningthat repeatedstart()calls produced in tests.Verification
pnpm --filter=@databricks/appkit typecheck— passvitest run server.test.ts— 30/30 pass (and the MaxListeners warning is gone)vitest run server.integration.test.ts— 8/8 passbiome checkon the changed file — cleanThis pull request and its description were written by Isaac.