Skip to content

fix: adjust keep alive and request timeout for webhook calls - #1317

Merged
itslenny merged 3 commits into
masterfrom
lenny/webhook-request-fixes
Aug 14, 2026
Merged

fix: adjust keep alive and request timeout for webhook calls#1317
itslenny merged 3 commits into
masterfrom
lenny/webhook-request-fixes

Conversation

@itslenny

@itslenny itslenny commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

What is the current behavior?

Keep alive timeout is capped at the request timeout. Most connections are re-established (and DNS re-resolved) from
scratch on every request. This causes Webhooks to occasionally fail with timeout of 4000ms exceeded before the request reaches its destination.

What is the new behavior?

  • Base keep alive timeout on the number of free sockets (no longer capped at the request
    timeout) so connections can actually be reused
  • Split headersTimeout/bodyTimeout from the overall request timeout, so a stalled response
    is distinguishable from a stalled connection
  • Add error cause to error message. The default error message gives no useful details.
    • Example: fetch failed becomes fetch failed (HeadersTimeoutError: Headers Timeout Error)

@itslenny
itslenny requested a review from a team as a code owner August 12, 2026 21:47
@coveralls

coveralls commented Aug 12, 2026

Copy link
Copy Markdown

Coverage Report for CI Build 31816605321

Warning

Build has drifted: This PR's base is out of sync with its target branch, so coverage data may include unrelated changes.
Quick fix: rebase this PR. Learn more →

Coverage decreased (-0.08%) to 81.198%

Details

  • Coverage decreased (-0.08%) from the base build.
  • Patch coverage: 5 of 5 lines across 1 file are fully covered (100%).
  • 19 coverage regressions across 2 files.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

19 previously-covered lines in 2 files lost coverage.

File Lines Losing Coverage Coverage
src/internal/http/agent.ts 17 32.56%
src/storage/protocols/tus/s3-locker.ts 2 78.78%

Coverage Stats

Coverage Status
Relevant Lines: 12896
Covered Lines: 10892
Line Coverage: 84.46%
Relevant Branches: 7687
Covered Branches: 5821
Branch Coverage: 75.73%
Branches in Coverage %: Yes
Coverage Strength: 534.61 hits per line

💛 - Coveralls

@itslenny
itslenny force-pushed the lenny/webhook-request-fixes branch from f435221 to f4e2c61 Compare August 12, 2026 21:55
Comment thread src/storage/events/lifecycle/webhook.ts
Comment thread src/storage/events/lifecycle/webhook.ts
Comment thread src/storage/events/lifecycle/webhook.ts
@itslenny
itslenny force-pushed the lenny/webhook-request-fixes branch 3 times, most recently from 95f9e73 to f478e6a Compare August 13, 2026 00:05
@itslenny
itslenny force-pushed the lenny/webhook-request-fixes branch from f478e6a to 1202c95 Compare August 13, 2026 15:14
fenos pushed a commit that referenced this pull request Aug 13, 2026
Ported from #1317 (originally against src/storage/events/lifecycle/webhook.ts,
which moved to src/storage/events/webhooks/webhook.ts on this branch):
give the AbortSignal its own connect budget on top of WEBHOOK_TIMEOUT_MS so
the dispatcher headersTimeout/bodyTimeout can actually fire, and stop capping
the keep-alive timeout at the request timeout.
@itslenny itslenny changed the title fix: add dns cache for webhook calls and increase keep alive timeout fix: adjust keep alive and request timeout for webhook calls Aug 13, 2026
@itslenny
itslenny enabled auto-merge (squash) August 14, 2026 15:52
@itslenny
itslenny merged commit 28cb35c into master Aug 14, 2026
25 checks passed
@itslenny
itslenny deleted the lenny/webhook-request-fixes branch August 14, 2026 15:56
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.

4 participants