Skip to content

feat(webhook): add opt-in retries to IncomingWebhook and WebhookTrigger#2641

Merged
zimeg merged 48 commits into
mainfrom
eden/webhook-retries
Jul 8, 2026
Merged

feat(webhook): add opt-in retries to IncomingWebhook and WebhookTrigger#2641
zimeg merged 48 commits into
mainfrom
eden/webhook-retries

Conversation

@zimeg

@zimeg zimeg commented Jul 2, 2026

Copy link
Copy Markdown
Member

Summary

Adds opt-in retry support to @slack/webhook, mirroring the retryConfig convention already used by @slack/web-api's WebClient. Stacked on top of #2615 (the WebhookTrigger class), so this PR is scoped to just the retry change.

  • Both IncomingWebhook and WebhookTrigger accept a new retryConfig?: RetryOptions constructor option.
  • Adds retry-policies.ts with the same named policies exported by @slack/web-apifiveRetriesInFiveMinutes, tenRetriesInAboutThirtyMinutes, rapidRetryPolicy — re-exported from the package entry point.
  • Each send() is wrapped in p-retry using the configured policy.

Default: no retries ({ retries: 0 }) when retryConfig is unset — preserves today's behavior; retries are opt-in.

Retry conditions (transient failures only):

  • Server errors (5xx) → retry
  • Network errors with no response → retry
  • All client errors (4xx), including rate limits (429) → abort immediately

Unlike @slack/web-api, 429 responses are not retried here. That client blocks its request queue to honor the Retry-After header; the webhook clients have no such queue, so a blind retry would ignore the server's backoff. Rate-limit handling is left to the caller, and all 4xx responses are treated the same.

retryConfig is a transport-only option and is stripped from the posted payload (like agent).

Notes for reviewers

  • 429 abort is intentional and test-covered — see the does not retry a 429 even when a retry policy is set cases in both IncomingWebhook.test.ts and WebhookTrigger.test.ts. The rationale is above (no request queue to honor Retry-After).
  • @types/retry is pinned to 0.12.0 against retry ^0.13.1 — this matches @slack/web-api's own pinning. 0.12.0 is the latest published version of the type definitions, and the OperationOptions interface we extend for RetryOptions is unchanged across 0.12/0.13.
  • retry-policies.ts mirrors @slack/web-api's rather than sharing it, matching the existing per-package convention in this monorepo.

Consumed downstream by slackapi/slack-github-action#630, which passes its 0/5/10/RAPID input through to these policies.

Requirements

🤖 Generated with Claude Code

zimeg and others added 7 commits June 2, 2026 14:17
Add a new WebhookTrigger class that mirrors IncomingWebhook but handles
Workflow Builder webhook triggers which return JSON responses with
arbitrary payloads (vs plain text "ok" from incoming webhooks).

- Constructor takes URL + defaults (timeout, agent) — same pattern
- send() accepts arbitrary key-value payload, returns { ok, body }
- Reuses existing error infrastructure and User-Agent instrumentation
- Enables consumers like slack-github-action to use the SDK instead
  of raw fetch for WFB triggers

Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
Workflow Builder webhook trigger inputs are always string values.

Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
# Conflicts:
#	packages/webhook/package.json
#	packages/webhook/src/index.ts
@zimeg zimeg added semver:minor enhancement M-T: A feature request for new functionality pkg:webhook applies to `@slack/webhook` labels Jul 2, 2026
@zimeg zimeg self-assigned this Jul 2, 2026
@changeset-bot

changeset-bot Bot commented Jul 2, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 35d7096

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@slack/webhook Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@codecov

codecov Bot commented Jul 2, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 90.81633% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.06%. Comparing base (a5bd5c8) to head (35d7096).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2641      +/-   ##
==========================================
+ Coverage   88.97%   89.06%   +0.09%     
==========================================
  Files          64       65       +1     
  Lines       10363    10436      +73     
  Branches      467      471       +4     
==========================================
+ Hits         9220     9295      +75     
+ Misses       1122     1120       -2     
  Partials       21       21              
Flag Coverage Δ
cli-hooks 89.06% <90.81%> (+0.09%) ⬆️
cli-test 89.06% <90.81%> (+0.09%) ⬆️
logger 89.06% <90.81%> (+0.09%) ⬆️
oauth 89.06% <90.81%> (+0.09%) ⬆️
socket-mode 89.06% <90.81%> (+0.09%) ⬆️
web-api 89.06% <90.81%> (+0.09%) ⬆️
webhook 89.06% <90.81%> (+0.09%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

zimeg and others added 14 commits July 2, 2026 02:25
Default the payload to an empty object so send() can be called with no
arguments, POSTing an empty body. send({}) continues to work unchanged.

Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
Allow send() to be called with no arguments, POSTing an empty body.
send({}) continues to work unchanged.

Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
…webhook-retries

# Conflicts:
#	packages/webhook/src/WebhookTrigger.ts
The `= {}` default now lives in the WebhookTrigger PR; keep the @param
description unchanged here.

Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
Use a falsy check so an empty-string URL is rejected up front rather
than failing later at request time. Adds a guard test.

Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
Use a falsy check so an empty-string URL is rejected up front rather
than failing later at request time. Adds a guard test.

Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
…s test

Add a test that a 401 with { ok: false, error: 'invalid_auth' } rejects
with an HTTPError exposing the response status and body. Remove the
success test asserting workflow_run_id so success cases only assert
result.ok === true.

Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
WebhookTrigger throws the same coded errors as IncomingWebhook but had
no correspondingly named types. Add WebhookTriggerSendError /
WebhookTriggerHTTPError / WebhookTriggerRequestError aliases and export
them so consumers have properly-named errors to catch.

Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
…source

Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
zimeg added a commit that referenced this pull request Jul 6, 2026
The webhook clients have no request queue to honor a Retry-After header,
so a 429 cannot respect the server's backoff on retry. Treat all 4xx
responses uniformly and leave rate-limit handling to the caller; only
5xx responses are retried. Addresses review feedback on #2641.

Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
The webhook clients have no request queue to honor a Retry-After header,
so a 429 cannot respect the server's backoff on retry. Treat all 4xx
responses uniformly and leave rate-limit handling to the caller; only
5xx responses are retried. Addresses review feedback on #2641.

Co-Authored-By: William Bergamin <25348381+WilliamBergamin@users.noreply.github.com>
Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
@zimeg zimeg force-pushed the eden/webhook-retries branch from 56cf76e to 3d9a0af Compare July 6, 2026 22:44
zimeg and others added 2 commits July 6, 2026 23:47

@zimeg zimeg left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔭 Duplicated comments from changed code!

Comment thread packages/webhook/src/index.ts Outdated

export { addAppMetadata } from './instrument';

export { default as retryPolicies, RetryOptions } from './retry-policies';

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎁 note: Here we match the export pattern of the @slack/web-api package:

export { default as retryPolicies, RetryOptions } from './retry-policies';

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Praise for matching the pattern 🚀

zimeg and others added 16 commits July 7, 2026 13:10
Drop the { ok, body } wrapper so send() resolves to the parsed response
body itself, exposing ok/error without nesting or a duplicated ok field.

Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
Assert that metadata added via addAppMetadata is sent in the User-Agent
header on a WebhookTrigger request, alongside the base webhook agent.

Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
…webhook-retries

# Conflicts:
#	packages/webhook/src/WebhookTrigger.test.ts
send() now returns response.data directly; the string-vs-object parse
branch was dead code since axios already parses JSON responses. Removes
the now-unused AxiosResponse import. Simplifies the eventual fetch swap.

Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
The trigger endpoint responds with { ok: true } or { ok: false, error },
so type the result closed rather than an open index signature. Widening
later is backward-compatible; tightening a released `any` would not be.

Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
…webhook-retries

# Conflicts:
#	packages/webhook/src/WebhookTrigger.ts
The package now covers both Incoming Webhooks and Workflow Builder
triggers, so the narrower "Incoming Webhooks" title is outdated.

Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
@zimeg zimeg requested a review from WilliamBergamin July 8, 2026 12:58

@WilliamBergamin WilliamBergamin left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ship it 🚀

if (error.response !== undefined) {
const status: number = error.response.status;
const wrapped = httpErrorWithOriginal(error);
throw status >= 500 ? wrapped : new AbortError(wrapped);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice 💯 thanks for simplifying this 🚀


export { addAppMetadata } from './instrument';

export { default as retryPolicies, RetryOptions } from './retry-policies';

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Praise for matching the pattern 🚀

if (error.response !== undefined) {
const status: number = error.response.status;
const wrapped = httpErrorWithOriginal(error);
throw status >= 500 ? wrapped : new AbortError(wrapped);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Praise 💯

Base automatically changed from eden/webhook-trigger to main July 8, 2026 17:48
# Conflicts:
#	packages/webhook/README.md
#	packages/webhook/package.json
#	packages/webhook/src/WebhookTrigger.test.ts
#	packages/webhook/src/WebhookTrigger.ts
#	packages/webhook/src/index.ts
@zimeg

zimeg commented Jul 8, 2026

Copy link
Copy Markdown
Member Author

@WilliamBergamin Will do! 🫡 I look forward to bringing this to the Slack GitHub Action after an upcoming release.

@zimeg zimeg merged commit a2f6e77 into main Jul 8, 2026
14 checks passed
@zimeg zimeg deleted the eden/webhook-retries branch July 8, 2026 20:26
WilliamBergamin added a commit that referenced this pull request Jul 9, 2026
Bring the new @slack/webhook features from main into v8:
- WebhookTrigger class for Workflow Builder triggers (#2615)
- opt-in retries for IncomingWebhook and WebhookTrigger (#2641)

Re-expressed on v8's fetch + Error-subclass architecture (main built them
on axios + error-factory functions). Non-webhook release churn from main's
parallel 7.x line (docs, web-api version bumps, CHANGELOGs) resolved to v8.

Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement M-T: A feature request for new functionality pkg:webhook applies to `@slack/webhook` semver:minor

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants