Skip to content

executors: add 30-second HTTP timeout to webhook executor#12

Open
jaredzwick wants to merge 1 commit into
mainfrom
hir112-webhook-timeout
Open

executors: add 30-second HTTP timeout to webhook executor#12
jaredzwick wants to merge 1 commit into
mainfrom
hir112-webhook-timeout

Conversation

@jaredzwick

Copy link
Copy Markdown
Collaborator

Summary

Follow-up from HIR-112 / PR #8 review.

  • process_actions now builds the shared reqwest::Client with a 30-second timeout (connection + read combined) via Client::builder().timeout(Duration::from_secs(30)).build(). A hung webhook target can no longer stall the executor loop indefinitely.
  • Added WebhookError::Timeout variant. reqwest::Error::is_timeout() maps to it rather than the generic Transport, so callers can distinguish timeout retries from unrecoverable transport errors.
  • Added wiremock test times_out_on_slow_target: mock responds after 2 s against a 1-second client timeout and asserts WebhookError::Timeout is returned.
  • README Action Executors → Webhook section documents the default 30-second timeout.

Test plan

  • cargo test --package pypes --lib executors — 17 tests pass, including new times_out_on_slow_target
  • No clippy warnings introduced in changed files
  • Pre-existing clippy issues in src/server/ are untouched (out of scope)

Files changed

  • src/executors/webhook.rsWebhookError::Timeout variant, map_transport helper, new test
  • src/executors/mod.rs — client builder with 30s timeout
  • README.md — timeout documentation

🤖 Generated with Claude Code

- Build the shared reqwest::Client in process_actions with a 30-second
  timeout so a hung webhook target cannot stall the executor loop.
- Add WebhookError::Timeout variant; map reqwest::Error::is_timeout()
  to it (distinct from generic Transport) so callers can retry on their
  own schedule.
- Add wiremock test times_out_on_slow_target: 2-second delay vs 1-second
  client timeout asserts WebhookError::Timeout is returned.
- Document default timeout in README Action Executors → Webhook section.

Closes HIR-112

Co-Authored-By: Paperclip <noreply@paperclip.ing>
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.

1 participant