From 0400bded685d0b3bde8bee72b4edf264ffd2d45a Mon Sep 17 00:00:00 2001 From: Isabel Wu <231155141+wuisabel-gif@users.noreply.github.com> Date: Tue, 7 Jul 2026 22:57:27 -0700 Subject: [PATCH] fix(webhook): don't let a bind failure take down the whole host MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The webhook server calls server.listen() with no 'error' handler, so if the port is already in use (a stray process, or a second nanoclaw copy) the EADDRINUSE surfaces as an uncaught exception and the entire host goes down — message polling, delivery and the sweep included, over one busy port. Handle 'error' instead: log the port and a hint, and keep the host up. The webhook-backed adapters stay dark until the conflict clears, but nothing else has to. Fixes #2900 --- src/webhook-server.test.ts | 30 +++++++++++++++++++++++++++++- src/webhook-server.ts | 17 +++++++++++++++++ 2 files changed, 46 insertions(+), 1 deletion(-) diff --git a/src/webhook-server.test.ts b/src/webhook-server.test.ts index b77da5b8304..3cf43a64f98 100644 --- a/src/webhook-server.test.ts +++ b/src/webhook-server.test.ts @@ -8,10 +8,13 @@ * route byte-identical. Conventions follow PR #2617: real HTTP server on a * fixed WEBHOOK_PORT, real fetch. */ -import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import http from 'http'; + +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; import type { Chat } from 'chat'; +import { log } from './log.js'; import { registerWebhookAdapter, stopWebhookServer } from './webhook-server.js'; const PORT = 3917; @@ -104,3 +107,28 @@ describe('registerWebhookAdapter — route/handler split', () => { expect(res.status).toBe(404); }); }); + +describe('registerWebhookAdapter — bind failure', () => { + it('EADDRINUSE is logged, not thrown as an uncaught exception that kills the host', async () => { + // Occupy the port the webhook server will try to bind. + const blocker = http.createServer(); + await new Promise((resolve) => blocker.listen(PORT, '0.0.0.0', resolve)); + const errSpy = vi.spyOn(log, 'error').mockImplementation(() => {}); + + try { + registerWebhookAdapter(stubChat('doomed').chat, 'slack'); + + // The 'error' event fires asynchronously after listen(); wait for it. + for (let i = 0; i < 40 && errSpy.mock.calls.length === 0; i++) { + await new Promise((r) => setTimeout(r, 25)); + } + + expect(errSpy).toHaveBeenCalled(); + const [, ctx] = errSpy.mock.calls[0] as [string, { code?: string }]; + expect(ctx.code).toBe('EADDRINUSE'); + } finally { + errSpy.mockRestore(); + await new Promise((resolve) => blocker.close(() => resolve())); + } + }); +}); diff --git a/src/webhook-server.ts b/src/webhook-server.ts index a05a93300b1..55b6de0f286 100644 --- a/src/webhook-server.ts +++ b/src/webhook-server.ts @@ -159,6 +159,23 @@ function ensureServer(): void { } }); + // Without an 'error' listener a bind failure (e.g. EADDRINUSE) becomes an + // uncaught exception that takes the whole host down with it — polling, + // delivery, sweep and all. Better to log the port and keep running; only the + // webhook-backed adapters go quiet until the conflict is cleared. + server.on('error', (err: NodeJS.ErrnoException) => { + server = null; + log.error('Webhook server failed to start', { + port, + code: err.code, + err, + hint: + err.code === 'EADDRINUSE' + ? `Port ${port} is already in use (another process or nanoclaw copy). Set WEBHOOK_PORT to a free port.` + : undefined, + }); + }); + server.listen(port, '0.0.0.0', () => { log.info('Webhook server started', { port, adapters: [...routes.keys()] }); });