No process-level unhandledRejection/uncaughtException handlers — a single unexpected throw can crash the process

[prodbycorne]

Overview

src/index.js registers SIGTERM/SIGINT handlers for graceful shutdown, but nowhere in the codebase does the process register process.on('unhandledRejection', ...) or process.on('uncaughtException', ...). Winston is configured with exitOnError: false (src/logger.js), which only prevents the logger itself from crashing the process on a logging-transport error — it has no bearing on unrelated unhandled promise rejections or synchronous uncaught exceptions elsewhere in the app.

Several code paths already discussed in other issues in this batch (unhandled Redis rejections deep in repository code, a Promise.all in webhookDispatcher.dispatch() that rejects on any single target's failure, the node-cron callback in priceRefresh.js which does catch its own errors but whose imported dependencies could still throw synchronously during module init) mean an unanticipated exception is a realistic, not merely theoretical, occurrence. Node's default behavior for an unhandled promise rejection (in modern Node versions, matching the engines.node: >=20.9.0 requirement in package.json) is to crash the process. Today, that means a single unlucky unhandled rejection anywhere — including inside a background job's async callback that escapes its own try/catch due to a bug, or a WebSocket handler's async callback — takes down the entire API server, dropping all in-flight requests and disconnecting every WebSocket client, rather than being logged and isolated.

Requirements

• Register process.on('unhandledRejection', ...) and process.on('uncaughtException', ...) handlers in src/index.js that:
  • Log the error at error level with full stack trace via the existing logger.
  • For uncaughtException specifically (which Node's own documentation says leaves the process in an undefined state), perform a controlled shutdown (reuse the existing shutdown() function's cleanup: stop jobs, close the server, disconnect Redis) rather than either crashing immediately and uncontrolled, or — the worse alternative — silently swallowing it and continuing to run in a potentially corrupted state.
  • For unhandledRejection, log with enough context to identify the source, and make a deliberate, documented choice about whether to treat it as fatal (matching uncaughtException's behavior) or merely log-and-continue, given Node's evolving default behavior around unhandled rejections.
• Ensure these new handlers do not mask or duplicate the existing per-job try/catch error handling already present in priceRefresh.js and webhookRetryWorker.js — they are a last-resort safety net, not a replacement for those.
• Add a metric/counter (or at minimum a distinctly-tagged log line) for process-level crashes so they're operationally visible/alertable, distinct from routine logger.error calls already scattered through business logic.

Acceptance Criteria

• An intentionally-thrown unhandled rejection in a test harness is caught by the new handler, logged, and triggers a graceful shutdown rather than an uncontrolled crash or silent continuation.
• uncaughtException triggers the same graceful shutdown path (jobs stopped, server closed, Redis disconnected) before the process exits.
• The new handlers are covered by at least one test that simulates each failure mode and asserts the graceful-shutdown sequence runs.
• Existing shutdown behavior triggered by SIGTERM/SIGINT is unaffected and continues to pass any existing tests.
• The behavioral choice for unhandledRejection (fatal vs. non-fatal) is explicitly documented in code comments and in the PR description.

Additional Notes

More precise references

• src/index.js:52-62 (shutdown(signal)): confirmed the exact cleanup sequence — priceRefreshJob.stop(), webhookRetryWorker.stop(), require('./ws/PriceSubscriptionManager').stopHeartbeat(), server.close(), await cache.disconnect(), process.exit(0) — this is the sequence any new crash-handler should reuse.
• src/index.js:72-73: confirmed only SIGTERM/SIGINT are registered; no process.on('unhandledRejection', ...) or process.on('uncaughtException', ...) anywhere in the file or, per a full read of logger.js, anywhere in the logger module either.
• src/logger.js:109-114: confirmed exitOnError: false is set on the winston logger itself — this only governs winston's own internal transport-error handling (e.g. a broken file-transport write), and has no relationship to unrelated unhandled promise rejections/exceptions elsewhere in the app, exactly as the issue states.
• src/jobs/priceRefresh.js:14-27: confirmed the cron callback already has its own try/catch wrapping refreshAllCachedPrices() and alertsService.evaluateAll(), logging via logger.error('Scheduled price refresh failed', ...) on failure — this existing per-job safety net should not be duplicated or masked by new process-level handlers, matching the issue's own caution.
• src/jobs/webhookRetryWorker.js:11-30 (tick): confirmed similarly wrapped in try/catch/finally, with a running guard flag preventing overlapping ticks — another existing safety net to preserve.
• src/services/webhookDispatcher.js:175-177 (dispatch): confirmed Promise.all(targets.map((webhook) => deliverToWebhook(...))) — if any single target's deliverToWebhook call rejects (not just resolves with a failure-status delivery record, but actually throws/rejects as a promise), Promise.all rejects the whole batch; tracing through deliverToWebhook → attempt(), most internal errors are caught and turned into delivery-status updates rather than thrown, but deliveryRepo.create/deliveryRepo.update/webhookRepo.findById calls themselves could still reject on a genuine Redis outage, and nothing in dispatch()'s caller chain currently catches that — worth confirming exactly which call site invokes dispatch() uncaught (likely from route handlers already wrapped in try/catch/next(err), but also potentially from the future Airdrops never auto-expire against expiry_ledger — no reconciliation job checks current chain state #88 expiry job, which needs its own handling regardless of this issue).

Additional edge cases

• Node's default behavior for unhandled promise rejections has changed across major versions (from a DeprecationWarning to process-crashing-by-default as of Node 15+); given package.json's engines.node: >=20.9.0 constraint (confirmed via the issue text, not independently re-verified this pass), the crash-by-default behavior is guaranteed — meaning today, in production, an unhandled rejection already crashes the process silently with just a stack trace to stderr and no structured log entry via winston, since nothing intercepts it before Node's default handler runs.
• A crash during the new uncaughtException-triggered graceful shutdown (e.g. cache.disconnect() itself throwing because Redis is the very thing that's unhealthy) needs a bounded timeout/fallback process.exit(1) so the process doesn't hang indefinitely in a broken shutdown sequence — Node's own docs explicitly warn that continuing after uncaughtException risks further corruption, so the shutdown path itself must be defensive and time-boxed (e.g. setTimeout(() => process.exit(1), 5000).unref() as a forced-exit fallback race against the graceful shutdown() promise).
• WebSocket clients (priceWebSocket.attach(server), src/index.js:67) are explicitly called out in the issue as collateral damage from an uncontrolled crash — worth confirming whether PriceSubscriptionManager.stopHeartbeat() (already called in shutdown()) actually notifies connected clients of an impending disconnect (e.g. a WS close frame with a reason code) versus just stopping a server-side heartbeat timer and letting the TCP connections drop abruptly — a gracefully-closed WS connection is meaningfully better for client-side reconnect logic than an abrupt drop, though this may be pre-existing behavior out of this issue's strict scope.

Implementation sketch

// src/index.js, after existing SIGTERM/SIGINT registration
let shuttingDown = false;

function crashShutdown(reason, err) {
  if (shuttingDown) return; // avoid re-entrant shutdown if a second crash occurs mid-shutdown
  shuttingDown = true;
  logger.error(`Process crash: ${reason}`, { error: err?.message, stack: err?.stack });
  // metrics/log line distinctly tagged for alerting, per requirements
  logger.error('PROCESS_CRASH', { reason, fatal: true });

  const forceExit = setTimeout(() => process.exit(1), 5000);
  if (typeof forceExit.unref === 'function') forceExit.unref();

  shutdown(reason)().catch(() => process.exit(1));
}

process.on('uncaughtException', (err) => crashShutdown('uncaughtException', err));
process.on('unhandledRejection', (reason) => {
  const err = reason instanceof Error ? reason : new Error(String(reason));
  crashShutdown('unhandledRejection', err); // documented choice: treat as fatal, matching uncaughtException
});

Refactor shutdown(signal) slightly so it can be invoked programmatically (not just as an process.on callback factory) without double-registering signal handlers — the sketch above assumes shutdown(reason)() still works given its current curried shape (confirmed compatible with src/index.js:52-53's existing function shutdown(signal) { return async () => {...} } structure).

Test/reproduction plan

• Integration test: spawn the app in a child process (or use a test harness that can trigger process.emit('unhandledRejection', ...) directly against the in-process app under Jest), assert the logger received a PROCESS_CRASH-tagged error entry and that shutdown's cleanup steps (mocked priceRefreshJob.stop, webhookRetryWorker.stop, cache.disconnect) were each called exactly once.
• Same for uncaughtException.
• Assert existing SIGTERM/SIGINT-triggered shutdown tests (if any exist in test/) still pass unchanged — confirm the refactor to make shutdown callable both ways didn't break the signal-handler registration.
• Test the forced-exit fallback: mock cache.disconnect() to hang forever (never-resolving promise); assert process.exit(1) is still called after the timeout rather than the process hanging (this may need to run in a genuinely separate process to actually observe process.exit, since Jest can't easily intercept a real process.exit call in the same process without a mock).

Cross-references

• Missing production metrics for webhook delivery outcomes, retry-queue depth, and price-source failure rates #93 (production metrics) — the "metric/counter for process-level crashes" this issue requires is a natural first consumer of whatever /metrics endpoint and Prometheus counter infrastructure Missing production metrics for webhook delivery outcomes, retry-queue depth, and price-source failure rates #93 introduces; if Missing production metrics for webhook delivery outcomes, retry-queue depth, and price-source failure rates #93 lands first, this issue's crash-counter requirement should use it directly rather than inventing a separate ad hoc mechanism.
• Health check endpoint reports only Redis status — misses price-source, webhook-worker, and DB connectivity #92 (health check depth) — a process that's mid-graceful-shutdown after a crash should ideally report an unhealthy/degraded status via /health for the brief window between crash-detection and process exit, so a load balancer stops routing new traffic to it immediately; worth cross-referencing Health check endpoint reports only Redis status — misses price-source, webhook-worker, and DB connectivity #92's health-status design so this issue's shutdown path can flip whatever health-state variable Health check endpoint reports only Redis status — misses price-source, webhook-worker, and DB connectivity #92 introduces before beginning cleanup.
• Background jobs (price refresh, webhook retry worker) have no leader election — every horizontally-scaled replica runs them independently #98 (leader election) — a crashed leader replica needs its lease to expire promptly so a follower can take over; this issue's forced-exit timeout (ensuring the process actually dies rather than hanging in a zombie state) is directly relevant to bounding Background jobs (price refresh, webhook retry worker) have no leader election — every horizontally-scaled replica runs them independently #98's documented failover window.