From e20a0039b314f59f3c84ccbd259efd8cbb90dc94 Mon Sep 17 00:00:00 2001 From: pick-cat Date: Thu, 30 Jul 2026 01:21:12 +0800 Subject: [PATCH] fix(telegram): catch unhandled rejections in webhook and ingress worker shutdown (#100998) * fix(telegram): isolate webhook/ingress-worker shutdown errors and guard all cleanup - Wrap entire shutdown() body in outer try/catch so sync throws never produce unhandled rejections from fire-and-forget abort listeners. - bot.stop() rejection is caught and logged; finally block ensures closeTransportOnce(), noteWebhookStop(), and stopDiagnosticHeartbeat() always execute regardless of bot.stop() outcome. - closeTransportOnce() rejection is caught independently in the finally block so noteWebhookStop() and stopDiagnosticHeartbeat() are not skipped. - Startup failure path: bot.stop() and closeTransportOnce() get .catch(() => undefined) so cleanup rejections never mask the original startup error. - Both abort-listener call sites use consistent void shutdown() since the never-reject contract is now internal. - Ingress worker stop() shares an in-flight stopPromise for idempotency; worker.terminate() rejections are observed via .catch(() => undefined). Co-Authored-By: nebulacoder-v8.0 * fix(telegram): continue webhook cleanup after sync server.close throw Extract independently guarded shutdown phases so a synchronous server.close() failure still runs bot.stop, transport close, status, and diagnostic heartbeat cleanup. Add sync-throw regression coverage and a loopback real-behavior proof (negative/positive/valid). * fix(telegram): clear webhook CI lint, knip, and test-types Restore curly-safe drain timer clear, stop exporting the unused shutdown phases interface, and narrow the transport close spy for tsgo test types. * chore(telegram): drop committed webhook shutdown proof script * test(telegram): inline webhook shutdown proof cases without proof script * fix(telegram): drop stale reply-fence import after main rebase telegram-reply-fence.ts was deleted on main (f7786a16cfe) during the core drain refactor. The rebased webhook.ts no longer calls these functions; remove the dangling import so the module resolves. Co-Authored-By: nebulacoder-v8.0 * fix(telegram): continue webhook shutdown after phase failures Keep each fallible Telegram-owned teardown phase independent so an early failure cannot skip transport, ingress, status, or diagnostic cleanup. Co-authored-by: Pick-cat --------- Co-authored-by: nebulacoder-v8.0 Co-authored-by: Vincent Koc --- extensions/telegram/src/webhook.test.ts | 22 +++++++++++++++++ extensions/telegram/src/webhook.ts | 33 +++++++++++++++++++------ 2 files changed, 47 insertions(+), 8 deletions(-) diff --git a/extensions/telegram/src/webhook.test.ts b/extensions/telegram/src/webhook.test.ts index 9b9ca471d330..397f1241dbaf 100644 --- a/extensions/telegram/src/webhook.test.ts +++ b/extensions/telegram/src/webhook.test.ts @@ -920,6 +920,28 @@ describe("startTelegramWebhook", () => { } }); + it("continues webhook shutdown after bot stop fails", async () => { + const runtimeError = vi.fn(); + const setStatus = vi.fn(); + stopSpy.mockRejectedValueOnce(new Error("bot stop failed")); + + const started = await startTelegramWebhook({ + token: TELEGRAM_TOKEN, + port: 0, + secret: TELEGRAM_SECRET, + path: TELEGRAM_WEBHOOK_PATH, + spoolDir: requireWebhookSpoolDir(), + setStatus, + runtime: { log: vi.fn(), error: runtimeError, exit: vi.fn() }, + }); + + await expect(started.stop()).resolves.toBeUndefined(); + + expect(transportCloseSpies[0]).toHaveBeenCalledOnce(); + expect(setStatus).toHaveBeenLastCalledWith({ mode: "webhook", connected: false }); + expectMockMessageContains(runtimeError, "telegram webhook bot stop failed"); + }); + it("marks delivery accepted only after the durable enqueue commits", async () => { let releaseEnqueue: (() => void) | undefined; let markEnqueueStarted: (() => void) | undefined; diff --git a/extensions/telegram/src/webhook.ts b/extensions/telegram/src/webhook.ts index 3224a2df1650..7bb9dd417c47 100644 --- a/extensions/telegram/src/webhook.ts +++ b/extensions/telegram/src/webhook.ts @@ -529,24 +529,41 @@ export async function startTelegramWebhook(opts: { }); let webhookAdvertised = false; + const runShutdownPhase = async ( + label: string, + run: () => void | Promise, + ): Promise => { + try { + await run(); + } catch (err) { + runtime.error?.(`telegram webhook ${label} failed: ${formatErrorMessage(err)}`); + } + }; const shutdown = async () => { if (shutDown) { return; } - botAbortController.abort(); shutDown = true; + botAbortController.abort(); shutdownAbortController.abort(); - const ingressStopTask = webhookIngressMonitor?.stop(); + // Every fallible phase is isolated so one failed release cannot skip the + // remaining resources or reject the fire-and-forget abort hook. + const ingressMonitor = webhookIngressMonitor; webhookIngressMonitor = undefined; - server.close(); - await bot.stop(); + const ingressStopTask = ingressMonitor + ? runShutdownPhase("ingress stop", () => ingressMonitor.stop()) + : undefined; + await runShutdownPhase("server close", () => { + server.close(); + }); + await runShutdownPhase("bot stop", () => bot.stop()); // The webhook owns this transport because it resolved and injected it into // createTelegramBot; close once so abort/startup-failure paths cannot leak sockets. - await closeTransportOnce(); - await waitForWebhookIngressStop(ingressStopTask); - status.noteWebhookStop(); + await runShutdownPhase("transport close", closeTransportOnce); + await runShutdownPhase("ingress drain", () => waitForWebhookIngressStop(ingressStopTask)); + await runShutdownPhase("status update", () => status.noteWebhookStop()); if (diagnosticsEnabled) { - stopDiagnosticHeartbeat(); + await runShutdownPhase("diagnostics stop", () => stopDiagnosticHeartbeat()); } }; if (opts.abortSignal?.aborted) {