chore: Improve handling of 'expected' network errors from webhooks (#12599)

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);
+  });
+});

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<Props> {
       });
       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,