diff --git a/.changeset/webhook-opt-in-retries.md b/.changeset/webhook-opt-in-retries.md new file mode 100644 index 000000000..11d1db909 --- /dev/null +++ b/.changeset/webhook-opt-in-retries.md @@ -0,0 +1,5 @@ +--- +"@slack/webhook": minor +--- + +feat: add opt-in retries to `IncomingWebhook` and `WebhookTrigger` diff --git a/package-lock.json b/package-lock.json index ff59e88a2..8943a604c 100644 --- a/package-lock.json +++ b/package-lock.json @@ -6505,7 +6505,10 @@ "dependencies": { "@slack/types": "^2.20.1", "@types/node": ">=18", - "axios": "^1.16.0" + "@types/retry": "0.12.0", + "axios": "^1.16.0", + "p-retry": "^4", + "retry": "^0.13.1" }, "devDependencies": { "nock": "^14.0.6" diff --git a/packages/webhook/README.md b/packages/webhook/README.md index 35215c82f..c76cc1c5a 100644 --- a/packages/webhook/README.md +++ b/packages/webhook/README.md @@ -103,6 +103,23 @@ const trigger = new WebhookTrigger(url); --- +### Retry failed requests + +Both `IncomingWebhook` and `WebhookTrigger` can retry failed requests. Retries are **off by default**; pass a `retryConfig` to opt in. The package re-exports the same named policies as `@slack/web-api` on `retryPolicies`, or you can supply your own [`retry`](https://github.com/tim-kos/node-retry) options. + +```javascript +const { IncomingWebhook, retryPolicies } = require('@slack/webhook'); +const url = process.env.SLACK_WEBHOOK_URL; + +const webhook = new IncomingWebhook(url, { + retryConfig: retryPolicies.fiveRetriesInFiveMinutes, +}); +``` + +Only transient failures are retried: server errors (`5xx`) and network errors with no response. Client errors (`4xx`), including rate limits (`429`), fail immediately without a retry. + +--- + ### Proxy requests with a custom agent The webhook allows you to customize the HTTP diff --git a/packages/webhook/package.json b/packages/webhook/package.json index ea69e94e7..6d8cce589 100644 --- a/packages/webhook/package.json +++ b/packages/webhook/package.json @@ -37,13 +37,16 @@ "build:clean": "shx rm -rf ./dist", "docs": "npx typedoc --plugin typedoc-plugin-markdown", "prepack": "npm run build", - "test": "npm run build && node --test-reporter=spec --test-reporter-destination=stdout --test-reporter=junit --test-reporter-destination=test-results.xml --import tsx --test src/IncomingWebhook.test.ts src/WebhookTrigger.test.ts src/instrument.test.ts", - "test:coverage": "npm run build && node --experimental-test-coverage --test-reporter=spec --test-reporter-destination=stdout --test-reporter=lcov --test-reporter-destination=lcov.info --test-reporter=junit --test-reporter-destination=test-results.xml --import tsx --test src/IncomingWebhook.test.ts src/WebhookTrigger.test.ts src/instrument.test.ts" + "test": "npm run build && node --test-reporter=spec --test-reporter-destination=stdout --test-reporter=junit --test-reporter-destination=test-results.xml --import tsx --test src/IncomingWebhook.test.ts src/WebhookTrigger.test.ts src/instrument.test.ts src/retry-policies.test.ts", + "test:coverage": "npm run build && node --experimental-test-coverage --test-reporter=spec --test-reporter-destination=stdout --test-reporter=lcov --test-reporter-destination=lcov.info --test-reporter=junit --test-reporter-destination=test-results.xml --import tsx --test src/IncomingWebhook.test.ts src/WebhookTrigger.test.ts src/instrument.test.ts src/retry-policies.test.ts" }, "dependencies": { "@slack/types": "^2.20.1", "@types/node": ">=18", - "axios": "^1.16.0" + "@types/retry": "0.12.0", + "axios": "^1.16.0", + "p-retry": "^4.6.2", + "retry": "^0.13.1" }, "devDependencies": { "nock": "^14.0.6" diff --git a/packages/webhook/src/IncomingWebhook.test.ts b/packages/webhook/src/IncomingWebhook.test.ts index 36db5946b..96dfafa0f 100644 --- a/packages/webhook/src/IncomingWebhook.test.ts +++ b/packages/webhook/src/IncomingWebhook.test.ts @@ -5,6 +5,7 @@ import nock from 'nock'; import type { CodedError } from './errors'; import { ErrorCode } from './errors'; import { IncomingWebhook } from './IncomingWebhook'; +import { rapidRetryPolicy } from './retry-policies'; const url = 'https://hooks.slack.com/services/FAKEWEBHOOK'; @@ -136,5 +137,78 @@ describe('IncomingWebhook', () => { } }); }); + + describe('retries', () => { + it('retries a 5xx then succeeds when a retry policy is set', async () => { + const scope = nock('https://hooks.slack.com') + .post(/services/) + .reply(503) + .post(/services/) + .reply(200, 'ok'); + const webhook = new IncomingWebhook(url, { retryConfig: rapidRetryPolicy }); + const result = await webhook.send('hello'); + assert.strictEqual(result.text, 'ok'); + scope.done(); + }); + + it('does not retry a 4xx even when a retry policy is set', async () => { + const scope = nock('https://hooks.slack.com') + .post(/services/) + .reply(400); + const webhook = new IncomingWebhook(url, { retryConfig: rapidRetryPolicy }); + try { + await webhook.send('hello'); + assert.fail('expected rejection'); + } catch (error) { + assert.strictEqual((error as CodedError).code, ErrorCode.HTTPError); + } + scope.done(); + }); + + it('does not retry a 429 even when a retry policy is set', async () => { + // Only one interceptor is registered; a retry would leave it unmatched + // and scope.done() would throw. + const scope = nock('https://hooks.slack.com') + .post(/services/) + .reply(429); + const webhook = new IncomingWebhook(url, { retryConfig: rapidRetryPolicy }); + try { + await webhook.send('hello'); + assert.fail('expected rejection'); + } catch (error) { + assert.strictEqual((error as CodedError).code, ErrorCode.HTTPError); + } + scope.done(); + }); + + it('does not retry by default (no retryConfig)', async () => { + const scope = nock('https://hooks.slack.com') + .post(/services/) + .reply(503); + const webhook = new IncomingWebhook(url); + try { + await webhook.send('hello'); + assert.fail('expected rejection'); + } catch (error) { + assert.strictEqual((error as CodedError).code, ErrorCode.HTTPError); + } + scope.done(); + }); + + it('does not leak retryConfig into the posted payload', async () => { + let posted: unknown; + const scope = nock('https://hooks.slack.com') + .post(/services/, (body) => { + posted = body; + return true; + }) + .reply(200, 'ok'); + const webhook = new IncomingWebhook(url, { retryConfig: rapidRetryPolicy }); + await webhook.send('hello'); + assert.ok(posted && typeof posted === 'object'); + assert.ok(!('retryConfig' in (posted as Record))); + scope.done(); + }); + }); }); }); diff --git a/packages/webhook/src/IncomingWebhook.ts b/packages/webhook/src/IncomingWebhook.ts index 8b5046121..e9160664a 100644 --- a/packages/webhook/src/IncomingWebhook.ts +++ b/packages/webhook/src/IncomingWebhook.ts @@ -2,9 +2,11 @@ import type { Agent } from 'node:http'; import type { Block, KnownBlock, MessageAttachment } from '@slack/types'; // TODO: Block and KnownBlock will be merged into AnyBlock in upcoming types release import axios, { type AxiosInstance, type AxiosResponse } from 'axios'; +import pRetry, { AbortError } from 'p-retry'; import { httpErrorWithOriginal, requestErrorWithOriginal } from './errors'; import { getUserAgent } from './instrument'; +import type { RetryOptions } from './retry-policies'; /** * A client for Slack's Incoming Webhooks @@ -25,6 +27,11 @@ export class IncomingWebhook { */ private axios: AxiosInstance; + /** + * Retry policy applied to each send. Defaults to no retries. + */ + private retryConfig: RetryOptions; + public constructor( url: string, defaults: IncomingWebhookDefaultArguments = { @@ -37,6 +44,7 @@ export class IncomingWebhook { this.url = url; this.defaults = defaults; + this.retryConfig = defaults.retryConfig ?? { retries: 0 }; this.axios = axios.create({ baseURL: url, @@ -50,7 +58,9 @@ export class IncomingWebhook { }, }); + // Strip transport-only options so they do not leak into the posted payload. this.defaults.agent = undefined; + this.defaults.retryConfig = undefined; } /** @@ -67,20 +77,25 @@ export class IncomingWebhook { payload = Object.assign(payload, message); } - try { - const response = await this.axios.post(this.url, payload); - return this.buildResult(response); - // biome-ignore lint/suspicious/noExplicitAny: errors can be anything - } catch (error: any) { - // Wrap errors in this packages own error types (abstract the implementation details' types) - if (error.response !== undefined) { - throw httpErrorWithOriginal(error); + return pRetry(async () => { + try { + const response = await this.axios.post(this.url, payload); + return this.buildResult(response); + // biome-ignore lint/suspicious/noExplicitAny: errors can be anything + } catch (error: any) { + // Wrap errors in this packages own error types (abstract the implementation details' types) + if (error.response !== undefined) { + const status: number = error.response.status; + const wrapped = httpErrorWithOriginal(error); + throw status >= 500 ? wrapped : new AbortError(wrapped); + } + if (error.request !== undefined) { + // No response received (network/timeout): retryable. + throw requestErrorWithOriginal(error); + } + throw new AbortError(error); } - if (error.request !== undefined) { - throw requestErrorWithOriginal(error); - } - throw error; - } + }, this.retryConfig); } /** @@ -105,6 +120,7 @@ export interface IncomingWebhookDefaultArguments { text?: string; link_names?: boolean; agent?: Agent; + retryConfig?: RetryOptions; timeout?: number; } diff --git a/packages/webhook/src/WebhookTrigger.test.ts b/packages/webhook/src/WebhookTrigger.test.ts index 61321df0c..362473052 100644 --- a/packages/webhook/src/WebhookTrigger.test.ts +++ b/packages/webhook/src/WebhookTrigger.test.ts @@ -5,6 +5,7 @@ import nock from 'nock'; import type { CodedError, WebhookTriggerHTTPError } from './errors'; import { ErrorCode } from './errors'; import { addAppMetadata } from './instrument'; +import { rapidRetryPolicy } from './retry-policies'; import { WebhookTrigger } from './WebhookTrigger'; const url = 'https://hooks.slack.com/triggers/FAKETRIGGER'; @@ -154,5 +155,83 @@ describe('WebhookTrigger', () => { } }); }); + + describe('retries', () => { + it('retries a 5xx then succeeds when a retry policy is set', async () => { + const scope = nock('https://hooks.slack.com') + .post(/triggers/) + .reply(503) + .post(/triggers/) + .reply(200, { ok: true }); + const trigger = new WebhookTrigger(url, { retryConfig: rapidRetryPolicy }); + const result = await trigger.send({ key: 'value' }); + assert.strictEqual(result.ok, true); + scope.done(); + }); + + it('does not retry a 4xx even when a retry policy is set', async () => { + const scope = nock('https://hooks.slack.com') + .post(/triggers/) + .reply(400); + const trigger = new WebhookTrigger(url, { retryConfig: rapidRetryPolicy }); + try { + await trigger.send({ key: 'value' }); + assert.fail('expected rejection'); + } catch (error) { + assert.strictEqual((error as CodedError).code, ErrorCode.HTTPError); + } + // Only one interceptor is registered; a retry would leave it unmatched + // and scope.done() would throw. + scope.done(); + }); + + it('does not retry a 429 even when a retry policy is set', async () => { + const scope = nock('https://hooks.slack.com') + .post(/triggers/) + .reply(429); + const trigger = new WebhookTrigger(url, { retryConfig: rapidRetryPolicy }); + try { + await trigger.send({ key: 'value' }); + assert.fail('expected rejection'); + } catch (error) { + assert.strictEqual((error as CodedError).code, ErrorCode.HTTPError); + } + // Only one interceptor is registered; a retry would leave it unmatched + // and scope.done() would throw. + scope.done(); + }); + + it('gives up with the HTTP error after exhausting retries', async () => { + const scope = nock('https://hooks.slack.com') + .post(/triggers/) + .reply(500) + .post(/triggers/) + .reply(500); + const trigger = new WebhookTrigger(url, { + retryConfig: { retries: 1, minTimeout: 0, maxTimeout: 1 }, + }); + try { + await trigger.send({ key: 'value' }); + assert.fail('expected rejection'); + } catch (error) { + assert.strictEqual((error as CodedError).code, ErrorCode.HTTPError); + } + scope.done(); + }); + + it('does not retry by default (no retryConfig)', async () => { + const scope = nock('https://hooks.slack.com') + .post(/triggers/) + .reply(503); + const trigger = new WebhookTrigger(url); + try { + await trigger.send({ key: 'value' }); + assert.fail('expected rejection'); + } catch (error) { + assert.strictEqual((error as CodedError).code, ErrorCode.HTTPError); + } + scope.done(); + }); + }); }); }); diff --git a/packages/webhook/src/WebhookTrigger.ts b/packages/webhook/src/WebhookTrigger.ts index 1b990bf0b..d50b772cc 100644 --- a/packages/webhook/src/WebhookTrigger.ts +++ b/packages/webhook/src/WebhookTrigger.ts @@ -1,9 +1,11 @@ import type { Agent } from 'node:http'; import axios, { type AxiosInstance } from 'axios'; +import pRetry, { AbortError } from 'p-retry'; import { httpErrorWithOriginal, requestErrorWithOriginal } from './errors'; import { getUserAgent } from './instrument'; +import type { RetryOptions } from './retry-policies'; /** * A client for Slack's Workflow Builder webhook triggers @@ -25,6 +27,11 @@ export class WebhookTrigger { */ private axios: AxiosInstance; + /** + * Retry policy applied to each send. Defaults to no retries. + */ + private retryConfig: RetryOptions; + public constructor( url: string, defaults: WebhookTriggerDefaultArguments = { @@ -37,6 +44,7 @@ export class WebhookTrigger { this.url = url; this.defaults = defaults; + this.retryConfig = defaults.retryConfig ?? { retries: 0 }; this.axios = axios.create({ baseURL: url, @@ -59,19 +67,24 @@ export class WebhookTrigger { * @param payload - arbitrary key-value data to send to the trigger */ public async send(payload: WebhookTriggerSendArguments = {}): Promise { - try { - const response = await this.axios.post(this.url, payload); - return response.data; - // biome-ignore lint/suspicious/noExplicitAny: errors can be anything - } catch (error: any) { - if (error.response !== undefined) { - throw httpErrorWithOriginal(error); + return pRetry(async () => { + try { + const response = await this.axios.post(this.url, payload); + return response.data; + // biome-ignore lint/suspicious/noExplicitAny: errors can be anything + } catch (error: any) { + if (error.response !== undefined) { + const status: number = error.response.status; + const wrapped = httpErrorWithOriginal(error); + throw status >= 500 ? wrapped : new AbortError(wrapped); + } + if (error.request !== undefined) { + // No response received (network/timeout): retryable. + throw requestErrorWithOriginal(error); + } + throw new AbortError(error); } - if (error.request !== undefined) { - throw requestErrorWithOriginal(error); - } - throw error; - } + }, this.retryConfig); } } @@ -81,6 +94,7 @@ export class WebhookTrigger { export interface WebhookTriggerDefaultArguments { agent?: Agent; + retryConfig?: RetryOptions; timeout?: number; } diff --git a/packages/webhook/src/index.ts b/packages/webhook/src/index.ts index 7f5b4d098..4a28e78bf 100644 --- a/packages/webhook/src/index.ts +++ b/packages/webhook/src/index.ts @@ -20,6 +20,8 @@ export { export { addAppMetadata } from './instrument'; +export { default as retryPolicies, RetryOptions } from './retry-policies'; + export { WebhookTrigger, WebhookTriggerDefaultArguments, diff --git a/packages/webhook/src/retry-policies.test.ts b/packages/webhook/src/retry-policies.test.ts new file mode 100644 index 000000000..4039ca6cd --- /dev/null +++ b/packages/webhook/src/retry-policies.test.ts @@ -0,0 +1,19 @@ +import assert from 'node:assert/strict'; +import { describe, it } from 'node:test'; + +import { fiveRetriesInFiveMinutes, rapidRetryPolicy, tenRetriesInAboutThirtyMinutes } from './retry-policies'; + +describe('retry-policies', () => { + it('fiveRetriesInFiveMinutes retries five times', () => { + assert.strictEqual(fiveRetriesInFiveMinutes.retries, 5); + }); + + it('tenRetriesInAboutThirtyMinutes retries ten times', () => { + assert.strictEqual(tenRetriesInAboutThirtyMinutes.retries, 10); + }); + + it('rapidRetryPolicy uses a tiny timeout', () => { + assert.strictEqual(rapidRetryPolicy.minTimeout, 0); + assert.strictEqual(rapidRetryPolicy.maxTimeout, 1); + }); +}); diff --git a/packages/webhook/src/retry-policies.ts b/packages/webhook/src/retry-policies.ts new file mode 100644 index 000000000..41a57a7e9 --- /dev/null +++ b/packages/webhook/src/retry-policies.ts @@ -0,0 +1,41 @@ +import type { OperationOptions } from 'retry'; + +/** + * Options to create retry policies. Extends from https://github.com/tim-kos/node-retry. + */ +export interface RetryOptions extends OperationOptions {} + +/** + * The default retry policy. Retry up to 10 times, over the span of about 30 minutes. It's not exact because + * randomization has been added to prevent a stampeding herd problem (if all instances in your application are retrying + * a request at the exact same intervals, they are more likely to cause failures for each other). + */ +export const tenRetriesInAboutThirtyMinutes: RetryOptions = { + retries: 10, + factor: 1.96821, + randomize: true, +}; + +/** + * Short & sweet, five retries in five minutes and then bail. + */ +export const fiveRetriesInFiveMinutes: RetryOptions = { + retries: 5, + factor: 3.86, +}; + +/** + * This policy is just to keep the tests running fast. + */ +export const rapidRetryPolicy: RetryOptions = { + minTimeout: 0, + maxTimeout: 1, +}; + +const policies = { + tenRetriesInAboutThirtyMinutes, + fiveRetriesInFiveMinutes, + rapidRetryPolicy, +}; + +export default policies;