From 9ec6b8309d5d4b4a6992433ad15276342ae43495 Mon Sep 17 00:00:00 2001 From: Tom Moor Date: Fri, 5 Jun 2026 18:00:37 -0400 Subject: [PATCH] chore: Improve handling of 'expected' network errors from webhooks (#12599) --- .../server/tasks/DeliverWebhookTask.test.ts | 65 ++++++++++++++++++- .../server/tasks/DeliverWebhookTask.ts | 52 ++++++++++++++- 2 files changed, 115 insertions(+), 2 deletions(-) diff --git a/plugins/webhooks/server/tasks/DeliverWebhookTask.test.ts b/plugins/webhooks/server/tasks/DeliverWebhookTask.test.ts index 0381365aec..0b007ea601 100644 --- a/plugins/webhooks/server/tasks/DeliverWebhookTask.test.ts +++ b/plugins/webhooks/server/tasks/DeliverWebhookTask.test.ts @@ -1,3 +1,4 @@ +import { FetchError } from "node-fetch"; import { http, HttpResponse, @@ -12,7 +13,9 @@ import { buildWebhookSubscription, } from "@server/test/factories"; import type { UserEvent } from "@server/types"; -import DeliverWebhookTask from "./DeliverWebhookTask"; +import DeliverWebhookTask, { + isExpectedNetworkError, +} from "./DeliverWebhookTask"; const ip = "127.0.0.1"; @@ -243,3 +246,63 @@ describe("DeliverWebhookTask", () => { expect(delivery.responseBody).toEqual('{"message":"Failure"}'); }); }); + +describe("isExpectedNetworkError", () => { + test("treats node-fetch FetchError as expected", () => { + expect( + isExpectedNetworkError( + new FetchError("request to https://example.com failed", "system") + ) + ).toBe(true); + }); + + test("treats raw socket errors as expected", () => { + expect(isExpectedNetworkError(new Error("socket hang up"))).toBe(true); + expect( + isExpectedNetworkError( + Object.assign(new Error("read ECONNRESET"), { code: "ECONNRESET" }) + ) + ).toBe(true); + }); + + test("treats connection error codes as expected", () => { + for (const code of [ + "ECONNREFUSED", + "ETIMEDOUT", + "EHOSTUNREACH", + "ENOTFOUND", + "EAI_AGAIN", + ]) { + expect( + isExpectedNetworkError(Object.assign(new Error("boom"), { code })) + ).toBe(true); + } + }); + + test("treats invalid certificate errors as expected", () => { + expect( + isExpectedNetworkError( + Object.assign(new Error("self signed certificate"), { + code: "DEPTH_ZERO_SELF_SIGNED_CERT", + }) + ) + ).toBe(true); + }); + + test("treats the request timeout thrown by the fetch wrapper as expected", () => { + expect( + isExpectedNetworkError(new Error("Request timeout after 5000ms")) + ).toBe(true); + }); + + test("does not treat unrelated errors as expected", () => { + expect( + isExpectedNetworkError(new TypeError("undefined is not a function")) + ).toBe(false); + expect( + isExpectedNetworkError(new Error("Cannot read property foo of undefined")) + ).toBe(false); + expect(isExpectedNetworkError("socket hang up")).toBe(false); + expect(isExpectedNetworkError(undefined)).toBe(false); + }); +}); diff --git a/plugins/webhooks/server/tasks/DeliverWebhookTask.ts b/plugins/webhooks/server/tasks/DeliverWebhookTask.ts index f2b8c2e41b..7fe6f088f0 100644 --- a/plugins/webhooks/server/tasks/DeliverWebhookTask.ts +++ b/plugins/webhooks/server/tasks/DeliverWebhookTask.ts @@ -78,6 +78,56 @@ function assertUnreachable(event: never) { Logger.warn(`DeliverWebhookTask did not handle ${(event as Event).name}`); } +/** + * Node connection-level error codes that are expected when delivering to + * arbitrary, user-supplied webhook URLs. These indicate a misconfigured or + * unreachable destination rather than a bug in Outline. + */ +const expectedNetworkErrorCodes = new Set([ + "ECONNRESET", + "ECONNREFUSED", + "ECONNABORTED", + "ETIMEDOUT", + "EHOSTUNREACH", + "ENETUNREACH", + "ENOTFOUND", + "EAI_AGAIN", + "EPIPE", + "EPROTO", + "DEPTH_ZERO_SELF_SIGNED_CERT", + "SELF_SIGNED_CERT_IN_CHAIN", + "UNABLE_TO_VERIFY_LEAF_SIGNATURE", + "CERT_HAS_EXPIRED", + "ERR_TLS_CERT_ALTNAME_INVALID", +]); + +/** + * Determine whether an error thrown while delivering a webhook is an expected + * network failure caused by the user-supplied destination URL (connection + * reset, timeout, unreachable host, invalid certificate, etc) rather than an + * unexpected bug. Such failures are noisy and do not need error tracking. + * + * @param err The error that occurred during delivery. + * @returns true if the error is an expected network failure. + */ +export function isExpectedNetworkError(err: unknown): boolean { + if (err instanceof FetchError) { + return true; + } + if (err instanceof Error) { + const code = (err as NodeJS.ErrnoException).code; + if (code && expectedNetworkErrorCodes.has(code)) { + return true; + } + // node-fetch surfaces some low-level socket failures (and our fetch wrapper + // converts aborted requests into timeouts) without a structured code. + return /socket hang up|request timeout|network|ECONNRESET/i.test( + err.message + ); + } + return false; +} + type Props = { subscriptionId: string; event: Event; @@ -765,7 +815,7 @@ export default class DeliverWebhookTask extends BaseTask { }); status = response.ok ? "success" : "failed"; } catch (err) { - if (err instanceof FetchError && env.isCloudHosted) { + if (isExpectedNetworkError(err) && env.isCloudHosted) { Logger.warn(`Failed to send webhook: ${err.message}`, { event, deliveryId: delivery.id,