From 9061f7eab1cd76d7e4c3a1ccad0aa8417f79ebac Mon Sep 17 00:00:00 2001 From: larondo1234 Date: Mon, 28 Sep 2026 23:56:24 +0100 Subject: [PATCH] feat: make the webhook request timeout configurable (#644) --- docs/LISTENER-CONFIGURATION.md | 10 +++ listener/.env.example | 4 ++ listener/src/config-schema.test.ts | 27 +++++++ listener/src/config-schema.ts | 9 +++ listener/src/config.test.ts | 47 ++++++++++++ listener/src/config.ts | 18 +++++ .../services/retry-scheduler-webhook.test.ts | 48 ++++++++++++- listener/src/services/retry-scheduler.ts | 12 +++- .../services/webhook-delivery-service.test.ts | 55 ++++++++++++++ .../src/services/webhook-delivery-service.ts | 41 +++++++++-- .../src/services/webhook-retry-helper.test.ts | 43 ++++++++++- listener/src/services/webhook-retry-helper.ts | 72 +++++++++++-------- listener/src/services/webhook-sender.ts | 29 ++++++++ listener/src/types/index.ts | 5 ++ 14 files changed, 380 insertions(+), 40 deletions(-) diff --git a/docs/LISTENER-CONFIGURATION.md b/docs/LISTENER-CONFIGURATION.md index 03639f4e..ddc51842 100644 --- a/docs/LISTENER-CONFIGURATION.md +++ b/docs/LISTENER-CONFIGURATION.md @@ -222,6 +222,16 @@ Also used by the DB-backed retry scheduler for `baseDelayMs` / `multiplier` / `j | `RETRY_SCHEDULER_BATCH_SIZE` | integer | `10` | No (defaulted) | Jobs per tick | | `RETRY_MAX_DELAY_MS` | integer (ms) | `3600000` (1h) | No (defaulted) | Max backoff clamp for retry scheduler | +### Outbound webhook requests + +| Name | Type | Default | Required | Purpose / effect | +|------|------|---------|----------|------------------| +| `WEBHOOK_TIMEOUT_MS` | integer (ms) | `10000` | No (defaulted) | Timeout for outbound webhook POSTs, applied independently of other network operations | + +`WEBHOOK_TIMEOUT_MS` is validated at startup: non-numeric values abort startup with a `ConfigError`, and values below `1` or above `300000` (5 minutes) are rejected. When a webhook does not respond within the configured timeout the request is aborted and recorded as a distinct **timeout** failure (rather than a generic network or HTTP error), so it is visible separately in logs and retry handling. + +**Recommended (guidance):** leave the `10000` ms default for typical endpoints; lower it when the receiver is expected to be fast and you want to fail over sooner, and raise it (up to `300000`) only for endpoints with a known long processing time. + ### Scheduled notification scheduler | Name | Type | Default | Required | Purpose / effect | diff --git a/listener/.env.example b/listener/.env.example index c5f610b6..023dd6b1 100644 --- a/listener/.env.example +++ b/listener/.env.example @@ -190,6 +190,10 @@ RETRY_SCHEDULER_PROCESSOR_ID= # Number of retry jobs to process per scheduler tick. RETRY_SCHEDULER_BATCH_SIZE=10 +# Timeout for outbound webhook requests (ms). Applies to the webhook delivery +# path independently of Discord/scheduler timeouts. Must be 1..300000. +WEBHOOK_TIMEOUT_MS=10000 + # ----------------------------------------------------------------------------- # Scheduled Notification Scheduler # ----------------------------------------------------------------------------- diff --git a/listener/src/config-schema.test.ts b/listener/src/config-schema.test.ts index aa86dd32..b2050003 100644 --- a/listener/src/config-schema.test.ts +++ b/listener/src/config-schema.test.ts @@ -96,4 +96,31 @@ describe('Configuration Schema Validation (#694)', () => { const errors = ConfigurationSchemaValidator.validate(invalidConfig, APP_CONFIG_SCHEMA); expect(errors.some((e) => e.field === 'stellarRpcUrl' && e.message.includes('pattern'))).toBe(true); }); + + it('rejects an out-of-range retryScheduler.webhookTimeoutMs', () => { + const tooSmall = { + ...sampleValidConfig, + retryScheduler: { webhookTimeoutMs: 0 }, + }; + const tooLarge = { + ...sampleValidConfig, + retryScheduler: { webhookTimeoutMs: 300001 }, + }; + + const smallErrors = ConfigurationSchemaValidator.validate(tooSmall, APP_CONFIG_SCHEMA); + const largeErrors = ConfigurationSchemaValidator.validate(tooLarge, APP_CONFIG_SCHEMA); + + expect(smallErrors.some((e) => e.field === 'retryScheduler.webhookTimeoutMs')).toBe(true); + expect(largeErrors.some((e) => e.field === 'retryScheduler.webhookTimeoutMs')).toBe(true); + }); + + it('accepts a valid retryScheduler.webhookTimeoutMs', () => { + const config = { + ...sampleValidConfig, + retryScheduler: { webhookTimeoutMs: 10000 }, + }; + + const errors = ConfigurationSchemaValidator.validate(config, APP_CONFIG_SCHEMA); + expect(errors.some((e) => e.field === 'retryScheduler.webhookTimeoutMs')).toBe(false); + }); }); diff --git a/listener/src/config-schema.ts b/listener/src/config-schema.ts index 58882278..d57e1249 100644 --- a/listener/src/config-schema.ts +++ b/listener/src/config-schema.ts @@ -8,6 +8,8 @@ * - Validation errors identify the affected configuration field explicitly. */ +import { MAX_WEBHOOK_TIMEOUT_MS } from './services/webhook-delivery-service'; + export type FieldType = 'string' | 'number' | 'boolean' | 'array' | 'object'; export interface SchemaFieldRule { @@ -212,6 +214,13 @@ export const APP_CONFIG_SCHEMA: ConfigSchema = { batchSize: { type: 'number', min: 1 }, timingBufferMs: { type: 'number', min: 0 }, }, + retryScheduler: { + enabled: { type: 'boolean' }, + pollIntervalMs: { type: 'number', min: 1000 }, + lockTimeoutMs: { type: 'number', min: 1000 }, + batchSize: { type: 'number', min: 1 }, + webhookTimeoutMs: { type: 'number', min: 1, max: MAX_WEBHOOK_TIMEOUT_MS }, + }, rateLimit: { enabled: { type: 'boolean' }, windowMs: { type: 'number', min: 1000 }, diff --git a/listener/src/config.test.ts b/listener/src/config.test.ts index 4508ddc6..0435778d 100644 --- a/listener/src/config.test.ts +++ b/listener/src/config.test.ts @@ -518,4 +518,51 @@ describe('Config validation', () => { ); }); }); + + describe('WEBHOOK_TIMEOUT_MS', () => { + it('defaults to 10000 when unset', () => { + delete process.env.WEBHOOK_TIMEOUT_MS; + + expect(loadConfig().retryScheduler?.webhookTimeoutMs).toBe(10000); + }); + + it('loads a configured webhook timeout', () => { + process.env.WEBHOOK_TIMEOUT_MS = '2500'; + + expect(loadConfig().retryScheduler?.webhookTimeoutMs).toBe(2500); + }); + + it('rejects a non-numeric webhook timeout at parse time', () => { + process.env.WEBHOOK_TIMEOUT_MS = 'soon'; + + expect(() => loadConfig()).toThrow(ConfigError); + expect(() => loadConfig()).toThrow( + 'WEBHOOK_TIMEOUT_MS must be a valid integer, got "soon"' + ); + }); + + it('rejects a zero webhook timeout', () => { + process.env.WEBHOOK_TIMEOUT_MS = '0'; + + const config = loadConfig(); + expect(() => validateConfig(config)).toThrow(ConfigError); + expect(() => validateConfig(config)).toThrow('WEBHOOK_TIMEOUT_MS must be >= 1 ms'); + }); + + it('rejects a negative webhook timeout', () => { + process.env.WEBHOOK_TIMEOUT_MS = '-50'; + + const config = loadConfig(); + expect(() => validateConfig(config)).toThrow(ConfigError); + expect(() => validateConfig(config)).toThrow('WEBHOOK_TIMEOUT_MS must be >= 1 ms'); + }); + + it('rejects an absurdly large webhook timeout', () => { + process.env.WEBHOOK_TIMEOUT_MS = '9999999'; + + const config = loadConfig(); + expect(() => validateConfig(config)).toThrow(ConfigError); + expect(() => validateConfig(config)).toThrow('WEBHOOK_TIMEOUT_MS must be <= 300000 ms'); + }); + }); }); diff --git a/listener/src/config.ts b/listener/src/config.ts index 3dbdd3f6..98b9c904 100644 --- a/listener/src/config.ts +++ b/listener/src/config.ts @@ -9,6 +9,10 @@ import { parseLogLevel, } from './utils/logger'; import { DEFAULT_MAX_BODY_BYTES } from './middleware/body-limit'; +import { + DEFAULT_WEBHOOK_TIMEOUT_MS, + MAX_WEBHOOK_TIMEOUT_MS, +} from './services/webhook-delivery-service'; export class ConfigError extends Error { constructor(message: string) { @@ -200,6 +204,10 @@ function loadRetrySchedulerConfig(): RetrySchedulerOptions { multiplier: parseIntegerEnv('RETRY_MULTIPLIER', '2'), maxDelayMs: parseIntegerEnv('RETRY_MAX_DELAY_MS', String(60 * 60 * 1000)), jitter: trimEnv('RETRY_JITTER') !== 'false', + webhookTimeoutMs: parseIntegerEnv( + 'WEBHOOK_TIMEOUT_MS', + String(DEFAULT_WEBHOOK_TIMEOUT_MS) + ), }; } @@ -558,6 +566,16 @@ export function validateConfig(config: Config): void { `RETRY_SCHEDULER_BATCH_SIZE must be >= 1 (received: ${config.retryScheduler.batchSize}).`, ); } + if (config.retryScheduler.webhookTimeoutMs < 1) { + errors.push( + `WEBHOOK_TIMEOUT_MS must be >= 1 ms (received: ${config.retryScheduler.webhookTimeoutMs}).`, + ); + } else if (config.retryScheduler.webhookTimeoutMs > MAX_WEBHOOK_TIMEOUT_MS) { + errors.push( + `WEBHOOK_TIMEOUT_MS must be <= ${MAX_WEBHOOK_TIMEOUT_MS} ms ` + + `(received: ${config.retryScheduler.webhookTimeoutMs}).`, + ); + } } // ── Rate limiting ────────────────────────────────────────────────────────── diff --git a/listener/src/services/retry-scheduler-webhook.test.ts b/listener/src/services/retry-scheduler-webhook.test.ts index b4fbbb2a..8a8228cb 100644 --- a/listener/src/services/retry-scheduler-webhook.test.ts +++ b/listener/src/services/retry-scheduler-webhook.test.ts @@ -8,7 +8,7 @@ * - Successful retries are marked COMPLETED and logged */ -import { jest, describe, it, expect, beforeEach } from '@jest/globals'; +import { jest, describe, it, expect, beforeEach, afterEach } from '@jest/globals'; import { RetryScheduler, RETRY_SCHEDULER_DEFAULTS } from './retry-scheduler'; import { WebhookDeliveryService } from './webhook-delivery-service'; import { NotificationStatus, NotificationType } from '../types/scheduled-notification'; @@ -417,4 +417,50 @@ describe('RetryScheduler — webhook retry queue', () => { ); }); }); + + // ── Configurable webhook timeout ────────────────────────────────────────── + describe('webhook timeout configuration', () => { + const realFetch = global.fetch; + + afterEach(() => { + (global as any).fetch = realFetch; + }); + + it('applies the configured webhookTimeoutMs to the outbound request', async () => { + const notification = makeWebhookNotification(); + const repo = makeRepo({ + fetchDueRetries: jest.fn().mockImplementation(() => Promise.resolve([notification])), + }); + + let aborted = false; + (global as any).fetch = jest.fn().mockImplementation((_url: any, init: any) => + new Promise((_resolve, reject) => { + (init.signal as AbortSignal).addEventListener('abort', () => { + aborted = true; + const err = new Error('The operation was aborted.'); + err.name = 'AbortError'; + reject(err); + }); + }), + ); + + const scheduler = new RetryScheduler(repo, { + ...RETRY_SCHEDULER_DEFAULTS, + pollIntervalMs: 1000, + lockTimeoutMs: 1000, + batchSize: 1, + webhookTimeoutMs: 25, + }); + await scheduler.runOnce(); + + expect(aborted).toBe(true); + expect(repo.markAsFailedOrRetry).toHaveBeenCalledWith( + 10, + expect.objectContaining({ message: expect.stringContaining('timed out after 25ms') }), + expect.anything(), + expect.anything(), + expect.anything(), + ); + }); + }); }); diff --git a/listener/src/services/retry-scheduler.ts b/listener/src/services/retry-scheduler.ts index fa65685b..3d530002 100644 --- a/listener/src/services/retry-scheduler.ts +++ b/listener/src/services/retry-scheduler.ts @@ -4,7 +4,7 @@ import { generateRequestId } from '../utils/request-id'; import { ScheduledNotificationRepository } from './scheduled-notification-repository'; import { ScheduledNotification, NotificationStatus } from '../types/scheduled-notification'; import { DiscordNotificationService } from './discord-notification'; -import { WebhookDeliveryService } from './webhook-delivery-service'; +import { WebhookDeliveryService, DEFAULT_WEBHOOK_TIMEOUT_MS } from './webhook-delivery-service'; import { getWorkerManager } from './worker-manager'; export interface RetrySchedulerConfig { @@ -26,6 +26,11 @@ export interface RetrySchedulerConfig { maxDelayMs: number; /** Add ±25 % random jitter to prevent thundering herd. Default: true. */ jitter: boolean; + /** + * Timeout (ms) applied to outbound webhook requests (`WEBHOOK_TIMEOUT_MS`). + * Default: DEFAULT_WEBHOOK_TIMEOUT_MS. + */ + webhookTimeoutMs: number; } export const RETRY_SCHEDULER_DEFAULTS: RetrySchedulerConfig = { @@ -37,6 +42,7 @@ export const RETRY_SCHEDULER_DEFAULTS: RetrySchedulerConfig = { multiplier: 2, maxDelayMs: 60 * 60 * 1_000, jitter: true, + webhookTimeoutMs: DEFAULT_WEBHOOK_TIMEOUT_MS, }; /** @@ -89,7 +95,9 @@ export class RetryScheduler { this.processorId = this.config.processorId ?? `retry-${uuidv4()}`; this.repository = repository; this.discordService = discordService ?? null; - this.webhookDeliveryService = webhookDeliveryService ?? new WebhookDeliveryService(); + this.webhookDeliveryService = + webhookDeliveryService ?? + new WebhookDeliveryService({ timeoutMs: this.config.webhookTimeoutMs }); } async start(): Promise { diff --git a/listener/src/services/webhook-delivery-service.test.ts b/listener/src/services/webhook-delivery-service.test.ts index 4899fce6..60019169 100644 --- a/listener/src/services/webhook-delivery-service.test.ts +++ b/listener/src/services/webhook-delivery-service.test.ts @@ -272,4 +272,59 @@ describe('WebhookDeliveryService', () => { ); }); }); + + // ── Machine-readable failure classification ────────────────────────────── + + describe('failure classification', () => { + it('labels a request timeout as "timeout"', async () => { + mockSendWebhook.mockRejectedValue(makeAbortError()); + + const result = await service.deliver(TARGET_URL, PAYLOAD, REQUEST_ID); + + expect(result.failureReason).toBe('timeout'); + }); + + it('labels a TimeoutError as "timeout"', async () => { + const err = new Error('request timed out'); + err.name = 'TimeoutError'; + mockSendWebhook.mockRejectedValue(err); + + const result = await service.deliver(TARGET_URL, PAYLOAD, REQUEST_ID); + + expect(result.failureReason).toBe('timeout'); + }); + + it('labels a generic network error as "network"', async () => { + mockSendWebhook.mockRejectedValue(new Error('ECONNREFUSED')); + + const result = await service.deliver(TARGET_URL, PAYLOAD, REQUEST_ID); + + expect(result.failureReason).toBe('network'); + expect(result.failureReason).not.toBe('timeout'); + }); + + it('labels a 5xx response as "http_retryable"', async () => { + mockSendWebhook.mockResolvedValue(makeResponse(503, false)); + + const result = await service.deliver(TARGET_URL, PAYLOAD, REQUEST_ID); + + expect(result.failureReason).toBe('http_retryable'); + }); + + it('labels a 4xx response as "http_permanent"', async () => { + mockSendWebhook.mockResolvedValue(makeResponse(404, false)); + + const result = await service.deliver(TARGET_URL, PAYLOAD, REQUEST_ID); + + expect(result.failureReason).toBe('http_permanent'); + }); + + it('leaves failureReason undefined on success', async () => { + mockSendWebhook.mockResolvedValue(makeResponse(200)); + + const result = await service.deliver(TARGET_URL, PAYLOAD, REQUEST_ID); + + expect(result.failureReason).toBeUndefined(); + }); + }); }); diff --git a/listener/src/services/webhook-delivery-service.ts b/listener/src/services/webhook-delivery-service.ts index d38f74f9..337def4d 100644 --- a/listener/src/services/webhook-delivery-service.ts +++ b/listener/src/services/webhook-delivery-service.ts @@ -17,10 +17,29 @@ */ import logger from '../utils/logger'; -import { sendWebhook, WebhookSendOptions } from './webhook-sender'; +import { + sendWebhook, + WebhookSendOptions, + WebhookFailureReason, + isWebhookTimeoutError, +} from './webhook-sender'; + +/** + * Default timeout applied to an outbound webhook request when the operator has + * not configured one. This preserves the timeout the delivery service has + * always applied implicitly, and is the default for `WEBHOOK_TIMEOUT_MS`. + */ +export const DEFAULT_WEBHOOK_TIMEOUT_MS = 10_000; + +/** + * Upper bound for a configured webhook timeout. A larger value is almost + * certainly a unit mistake (e.g. milliseconds vs seconds) and would let a + * hung endpoint tie up a delivery worker for an unreasonable length of time. + */ +export const MAX_WEBHOOK_TIMEOUT_MS = 300_000; export interface WebhookDeliveryOptions { - /** Request timeout in milliseconds (default: 10 000). */ + /** Request timeout in milliseconds (default: DEFAULT_WEBHOOK_TIMEOUT_MS). */ timeoutMs?: number; /** Extra headers forwarded to every outbound request. */ headers?: Record; @@ -33,6 +52,12 @@ export interface WebhookDeliveryResult { statusCode?: number; /** Human-readable failure reason for logging. */ errorReason?: string; + /** + * Machine-readable failure classification. Distinguishes a request timeout + * from a generic network error or an HTTP-level failure. Undefined on + * success. + */ + failureReason?: WebhookFailureReason; } export class WebhookDeliveryService { @@ -40,7 +65,7 @@ export class WebhookDeliveryService { private readonly defaultHeaders: Record; constructor(options: WebhookDeliveryOptions = {}) { - this.defaultTimeoutMs = options.timeoutMs ?? 10_000; + this.defaultTimeoutMs = options.timeoutMs ?? DEFAULT_WEBHOOK_TIMEOUT_MS; this.defaultHeaders = options.headers ?? {}; } @@ -94,6 +119,7 @@ export class WebhookDeliveryService { success: false, statusCode: response.status, errorReason: `HTTP ${response.status}`, + failureReason: 'http_retryable', }; } @@ -107,10 +133,11 @@ export class WebhookDeliveryService { success: false, statusCode: response.status, errorReason: `HTTP ${response.status}`, + failureReason: 'http_permanent', }; } catch (err) { const durationMs = Date.now() - startMs; - const isTimeout = err instanceof Error && err.name === 'AbortError'; + const isTimeout = isWebhookTimeoutError(err); const errorReason = isTimeout ? `Webhook request timed out after ${timeoutMs}ms` : err instanceof Error @@ -131,7 +158,11 @@ export class WebhookDeliveryService { }); } - return { success: false, errorReason }; + return { + success: false, + errorReason, + failureReason: isTimeout ? 'timeout' : 'network', + }; } } } diff --git a/listener/src/services/webhook-retry-helper.test.ts b/listener/src/services/webhook-retry-helper.test.ts index 34cd74e2..bf85c1ce 100644 --- a/listener/src/services/webhook-retry-helper.test.ts +++ b/listener/src/services/webhook-retry-helper.test.ts @@ -15,7 +15,7 @@ */ import { jest, describe, it, expect, beforeEach, afterEach } from '@jest/globals'; -import { sendWebhookWithRetry } from './webhook-retry-helper'; +import { sendWebhookWithRetry, classifyWebhookFailure } from './webhook-retry-helper'; // --------------------------------------------------------------------------- // Mock sendWebhook to prevent real HTTP requests @@ -423,4 +423,45 @@ describe('sendWebhookWithRetry', () => { ); }); }); + + // ── Failure classification ──────────────────────────────────────────────── + + describe('classifyWebhookFailure', () => { + it('classifies an AbortError as a timeout', () => { + expect(classifyWebhookFailure(undefined, makeTimeoutError())).toBe('timeout'); + }); + + it('classifies a TimeoutError as a timeout', () => { + const err = new Error('request timed out'); + err.name = 'TimeoutError'; + + expect(classifyWebhookFailure(undefined, err)).toBe('timeout'); + }); + + it('classifies a generic error as a network failure (not a timeout)', () => { + expect(classifyWebhookFailure(undefined, makeNetworkError())).toBe('network'); + }); + + it('does not treat a message mentioning "timeout" as a timeout', () => { + // Only the error name marks an abort; a generic message must not be + // mistaken for a timeout, otherwise retry/observability cannot tell them apart. + expect(classifyWebhookFailure(undefined, new Error('timeout'))).toBe('network'); + }); + + it('classifies retryable HTTP statuses', () => { + for (const status of [429, 500, 502, 503, 504]) { + expect(classifyWebhookFailure(makeResponse(status, false))).toBe('http_retryable'); + } + }); + + it('classifies permanent client errors', () => { + for (const status of [400, 401, 403, 404, 422]) { + expect(classifyWebhookFailure(makeResponse(status, false))).toBe('http_permanent'); + } + }); + + it('returns null for a successful response', () => { + expect(classifyWebhookFailure(makeResponse(200))).toBeNull(); + }); + }); }); diff --git a/listener/src/services/webhook-retry-helper.ts b/listener/src/services/webhook-retry-helper.ts index 14bee3b9..88cb8ceb 100644 --- a/listener/src/services/webhook-retry-helper.ts +++ b/listener/src/services/webhook-retry-helper.ts @@ -14,7 +14,7 @@ * A delay is added between attempts to reduce load on failing services. */ -import { sendWebhook, WebhookSendOptions } from './webhook-sender'; +import { sendWebhook, WebhookSendOptions, WebhookFailureReason, isWebhookTimeoutError } from './webhook-sender'; /** Maximum number of retry attempts (not counting the initial attempt). */ const MAX_RETRY_ATTEMPTS = 2; @@ -33,45 +33,55 @@ const RETRYABLE_STATUS_CODES = new Set([429, 500, 502, 503, 504]); const PERMANENT_CLIENT_ERRORS = new Set([400, 401, 403, 404, 422]); /** - * Determines if an error or response should trigger a retry. + * Classify a webhook attempt as a specific failure reason, or `null` when it + * succeeded / did not fail. * - * @param response - The HTTP response, if available - * @param error - The error thrown, if any - * @returns true if the failure is retryable + * A request timeout is reported as its own `'timeout'` reason rather than + * being folded into a generic network error, so retry logic and observability + * can treat (and count) the two separately. + * + * @param response - The HTTP response, if one was received + * @param error - The error thrown, if the request failed before/without a response + * @returns the failure reason, or null when the attempt did not fail */ -function isRetryable(response?: Response, error?: unknown): boolean { - // Network errors and timeouts are retryable +export function classifyWebhookFailure( + response?: Response, + error?: unknown +): WebhookFailureReason | null { + // A thrown error means no HTTP response was available. Timeouts are their + // own reason; everything else is a network-level failure. if (error) { - return true; + return isWebhookTimeoutError(error) ? 'timeout' : 'network'; } - // Check HTTP status codes - if (response) { - // Success responses don't need retry - if (response.ok) { - return false; - } - - // Permanent client errors should not be retried - if (PERMANENT_CLIENT_ERRORS.has(response.status)) { - return false; - } - - // Explicit retryable status codes - if (RETRYABLE_STATUS_CODES.has(response.status)) { - return true; - } + if (!response || response.ok) { + return null; + } - // Any other 5xx error is retryable - if (response.status >= 500) { - return true; - } + // Permanent client errors are not worth retrying. + if (PERMANENT_CLIENT_ERRORS.has(response.status)) { + return 'http_permanent'; + } - // Other status codes (e.g., redirects, other 4xx) are not retried - return false; + // Explicit retryable status codes, plus any other 5xx. + if (RETRYABLE_STATUS_CODES.has(response.status) || response.status >= 500) { + return 'http_retryable'; } - return false; + // Other status codes (e.g. redirects, unlisted 4xx) are non-retryable. + return 'http_permanent'; +} + +/** + * Determines if an error or response should trigger a retry. + * + * @param response - The HTTP response, if available + * @param error - The error thrown, if any + * @returns true if the failure is retryable + */ +function isRetryable(response?: Response, error?: unknown): boolean { + const reason = classifyWebhookFailure(response, error); + return reason === 'timeout' || reason === 'network' || reason === 'http_retryable'; } /** diff --git a/listener/src/services/webhook-sender.ts b/listener/src/services/webhook-sender.ts index a3ca8306..e70c89f3 100644 --- a/listener/src/services/webhook-sender.ts +++ b/listener/src/services/webhook-sender.ts @@ -3,6 +3,35 @@ export interface WebhookSendOptions { headers?: Record; } +/** + * Classification of an outbound webhook failure. + * + * Kept deliberately coarse so that retry decisions and observability can tell + * a request timeout apart from a generic network error or an HTTP-level + * failure without string-matching error messages. + */ +export type WebhookFailureReason = + | 'timeout' + | 'network' + | 'http_retryable' + | 'http_permanent'; + +/** + * True when `error` represents an aborted (timed-out) webhook request. + * + * `AbortController.abort()` surfaces as an `AbortError` under Node's `fetch`; + * `TimeoutError` is accepted as well because some runtimes raise it for a + * signal-driven timeout. Callers use this instead of comparing + * `error.name === 'AbortError'` so a timeout stays distinguishable in one + * place. + */ +export function isWebhookTimeoutError(error: unknown): boolean { + return ( + error instanceof Error && + (error.name === 'AbortError' || error.name === 'TimeoutError') + ); +} + export async function sendWebhook( url: string, payload: any, diff --git a/listener/src/types/index.ts b/listener/src/types/index.ts index 1d212d13..bd551c84 100644 --- a/listener/src/types/index.ts +++ b/listener/src/types/index.ts @@ -135,6 +135,11 @@ export interface RetrySchedulerOptions { multiplier: number; maxDelayMs: number; jitter: boolean; + /** + * Timeout (ms) for outbound webhook requests (`WEBHOOK_TIMEOUT_MS`). + * Defaults to `DEFAULT_WEBHOOK_TIMEOUT_MS` (10 000 ms). + */ + webhookTimeoutMs: number; } export interface AnalyticsConfig {