Skip to content

feat: make the webhook request timeout configurable - #885

Merged
Abd-Standard merged 2 commits into
Core-Foundry:mainfrom
larondo1234:feat/webhook-timeout-config-644
Oct 3, 2026
Merged

Abd-Standard merged 2 commits into
Core-Foundry:mainfrom
larondo1234:feat/webhook-timeout-config-644

Conversation

@larondo1234

Copy link
Copy Markdown
Contributor

Overview

Makes the timeout for outbound webhook requests operator-configurable. Today the timeout that governs a webhook POST is hard-coded: sendWebhook() falls back to 5000 ms while the live webhook delivery path (RetryScheduler → WebhookDeliveryService) silently uses 10 000 ms, and there is no way to tune it without editing code. This adds a validated WEBHOOK_TIMEOUT_MS setting with a behaviour-preserving default, wires it through the existing AbortController path so the configured value actually governs the request, and surfaces a timeout as its own machine-readable failure reason — distinct from a generic network or HTTP failure.

Related Issue

Fixes the gap described in #644.

Changes

  • [MODIFY] listener/src/services/webhook-sender.ts

    • Adds WebhookFailureReason and isWebhookTimeoutError(), a single place that recognises an aborted/timed-out webhook request (AbortError/TimeoutError) instead of every caller string-matching error.name.
  • [MODIFY] listener/src/services/webhook-delivery-service.ts

    • Adds DEFAULT_WEBHOOK_TIMEOUT_MS (10000, the timeout the live path already applied implicitly) and MAX_WEBHOOK_TIMEOUT_MS (300000).
    • Uses the shared timeout constants and adds WebhookDeliveryResult.failureReason (timeout / network / http_retryable / http_permanent) so timeout failures are distinguishable programmatically, not only by message text.
  • [MODIFY] listener/src/services/webhook-retry-helper.ts

    • Adds classifyWebhookFailure() and routes isRetryable() through it. Retry behaviour is unchanged, but a timeout is now a distinct reason rather than being folded into "some error".
  • [MODIFY] listener/src/services/retry-scheduler.ts

    • Adds RetrySchedulerConfig.webhookTimeoutMs (defaulting to DEFAULT_WEBHOOK_TIMEOUT_MS) and constructs the internal WebhookDeliveryService with the configured timeout, so WEBHOOK_TIMEOUT_MS reaches the AbortController.
  • [MODIFY] listener/src/types/index.ts, listener/src/config.ts, listener/src/config-schema.ts

    • Loads WEBHOOK_TIMEOUT_MS via the existing integer parser (non-numeric values abort startup with ConfigError), rejects < 1 and > 300000 in validateConfig, and adds the field to APP_CONFIG_SCHEMA.
  • [MODIFY] listener/.env.example, docs/LISTENER-CONFIGURATION.md

    • Documents WEBHOOK_TIMEOUT_MS, its default, valid range, and the distinct timeout failure reason.
  • [MODIFY] listener/src/config.test.ts, listener/src/config-schema.test.ts, listener/src/services/webhook-delivery-service.test.ts, listener/src/services/webhook-retry-helper.test.ts, listener/src/services/retry-scheduler-webhook.test.ts

    • Tests for default applied when unset, configured value loaded, non-numeric/zero/negative/absurd values rejected, AbortError/TimeoutError classified as timeout, and the configured value governing a real (stubbed-fetch) request through the scheduler.

Verification Results

listener/node_modules is not installed and must not be installed (no jest/sqlite3 native build), so verification executes the dependency-free parts with the workspace tsx against the real modules:

$ node_modules/.bin/tsx scratch/tmp-146/harness.ts
RESULT: 29 passed, 0 failed
  - default webhookTimeoutMs is 10000; configured 2500 loaded
  - non-numeric / zero / negative / 300001 rejected (ConfigError)
  - AbortError + TimeoutError -> "timeout"; generic error -> "network";
    a message merely containing "timeout" is NOT a timeout
  - 429/503 -> http_retryable; 404 -> http_permanent; 2xx -> null
  - delivery with timeoutMs=60 aborts at ~60ms, errorReason
    "Webhook request timed out after 60ms", failureReason "timeout"
  - config-derived value (45ms) governs the request through WebhookDeliveryService

$ node_modules/.bin/tsx scratch/tmp-146/harness-scheduler.ts
RESULT: 8 passed, 0 failed
  - real RetryScheduler + real WebhookDeliveryService + stubbed fetch
  - configured webhookTimeoutMs=50 aborts the outbound request at ~50ms and
    records "Webhook request timed out after 50ms" (not a generic network error)
  - reconfigured 120ms is honoured on the next run

Focused parse/type checks (run with the workspace tsx/tsc; no installs):

$ esbuild --outfile=/dev/null <each changed .ts file>
OK for every changed file except config.test.ts, which fails with
"Unexpected end of file" on upstream main too (pre-existing unbalanced
describe; also confirmed against the GitHub API).

$ tsc --noEmit ... src/services/webhook-sender.ts webhook-delivery-service.ts webhook-retry-helper.ts
No errors in the changed files. Only pre-existing errors in src/utils/logger.ts
(winston has no type declarations in this checkout).

$ tsc --noEmit ... src/config.ts src/config-schema.ts
No errors from the WEBHOOK_TIMEOUT_MS additions. Pre-existing upstream errors
only: duplicate `./types` import identifiers, `Cannot find name 'validateSecrets'`
(config.ts calls it without importing it on main), and logger/winston.

$ tsc --noEmit ... src/services/retry-scheduler.ts
No errors from the webhook-timeout change. Pre-existing upstream error only:
`src/services/discord-notification.ts(472,1): '}' expected` (unbalanced function
on main).

Pre-existing upstream breakage encountered while verifying (unrelated to this change, reproduced against main via the GitHub API, and not modified here): listener/src/index.ts (syntax errors), listener/src/utils/request-id.ts (generateCorrelationId unbalanced), listener/src/services/discord-notification.ts (sanitizeForDiscord unbalanced), listener/src/config.ts (validateSecrets never imported), listener/src/config.test.ts (unclosed describe). The RetryScheduler harness was run against a work copy with a local-only repair of request-id.ts's syntax so the module could load; that repair is not part of this PR.

Acceptance Criteria Status
Webhook timeout is configurable ✅ WEBHOOK_TIMEOUT_MS → RetrySchedulerConfig.webhookTimeoutMs → WebhookDeliveryService → sendWebhook → AbortController
A sensible default is provided ✅ DEFAULT_WEBHOOK_TIMEOUT_MS = 10000, matching the timeout the live webhook path already applied
Invalid timeout values are rejected ✅ non-numeric aborts loadConfig; < 1 or > 300000 rejected by validateConfig + schema
Timeout failures are distinguishable from other failures ✅ failureReason: "timeout" + dedicated errorReason, separate from network / http_retryable / http_permanent

Closes #644

@drips-wave

drips-wave Bot commented Sep 28, 2026

Copy link
Copy Markdown

@larondo1234 Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits.

You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀

Learn more about application limits

Resolves merge conflicts against Core-Foundry/Notify-Chain@6d24241 (76 commit(s) behind) so the PR is mergeable.
@Abd-Standard
Abd-Standard merged commit 20328d2 into Core-Foundry:main Oct 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Make Webhook Timeout Configurable

2 participants