From 322414cbdd02205bff40dbf15fd4f4f0fe445f17 Mon Sep 17 00:00:00 2001 From: Eden Zimbelman Date: Tue, 30 Jun 2026 04:02:52 -0700 Subject: [PATCH 1/8] build(deps): use @slack/webhook + p-retry, drop axios Co-Authored-By: Claude --- package-lock.json | 90 ++++++++++++++++++++++++++++++----------------- package.json | 6 ++-- 2 files changed, 60 insertions(+), 36 deletions(-) diff --git a/package-lock.json b/package-lock.json index ad6e02a7..daac52a3 100644 --- a/package-lock.json +++ b/package-lock.json @@ -13,12 +13,12 @@ "@actions/github": "^9.1.1", "@slack/logger": "^4.0.1", "@slack/web-api": "^7.16.0", - "axios": "^1.16.0", - "axios-retry": "^4.5.0", + "@slack/webhook": "file:../node-slack-sdk/packages/webhook", "flat": "^6.0.1", "https-proxy-agent": "^9.0.0", "js-yaml": "^4.2.0", - "markup-js": "^1.5.21" + "markup-js": "^1.5.21", + "p-retry": "^8.0.0" }, "devDependencies": { "@biomejs/biome": "^2.4.16", @@ -37,6 +37,23 @@ "npm": ">=11.6.1" } }, + "../node-slack-sdk/packages/webhook": { + "name": "@slack/webhook", + "version": "7.0.9", + "license": "MIT", + "dependencies": { + "@slack/types": "^2.20.1", + "@types/node": ">=18", + "axios": "^1.16.0" + }, + "devDependencies": { + "nock": "^14.0.6" + }, + "engines": { + "node": ">= 18", + "npm": ">= 8.6.0" + } + }, "node_modules/@actions/core": { "version": "3.0.1", "resolved": "https://registry.npmjs.org/@actions/core/-/core-3.0.1.tgz", @@ -859,6 +876,23 @@ "npm": ">= 8.6.0" } }, + "node_modules/@slack/web-api/node_modules/p-retry": { + "version": "4.6.2", + "resolved": "https://registry.npmjs.org/p-retry/-/p-retry-4.6.2.tgz", + "integrity": "sha512-312Id396EbJdvRONlngUx0NydfrIQ5lsYu0znKVUzVvArzEIt08V1qhtyESbGVd1FGX7UKtiFp5uwKZdM8wIuQ==", + "license": "MIT", + "dependencies": { + "@types/retry": "0.12.0", + "retry": "^0.13.1" + }, + "engines": { + "node": ">=8" + } + }, + "node_modules/@slack/webhook": { + "resolved": "../node-slack-sdk/packages/webhook", + "link": true + }, "node_modules/@types/flat": { "version": "5.0.5", "resolved": "https://registry.npmjs.org/@types/flat/-/flat-5.0.5.tgz", @@ -983,18 +1017,6 @@ "proxy-from-env": "^2.1.0" } }, - "node_modules/axios-retry": { - "version": "4.5.0", - "resolved": "https://registry.npmjs.org/axios-retry/-/axios-retry-4.5.0.tgz", - "integrity": "sha512-aR99oXhpEDGo0UuAlYcn2iGRds30k366Zfa05XWScR9QaQD4JYiP3/1Qt1u7YlefUOK+cn0CcwoL1oefavQUlQ==", - "license": "Apache-2.0", - "dependencies": { - "is-retry-allowed": "^2.2.0" - }, - "peerDependencies": { - "axios": "0.x || 1.x" - } - }, "node_modules/before-after-hook": { "version": "4.0.0", "resolved": "https://registry.npmjs.org/before-after-hook/-/before-after-hook-4.0.0.tgz", @@ -1590,6 +1612,18 @@ "node": ">=0.10.0" } }, + "node_modules/is-network-error": { + "version": "1.3.2", + "resolved": "https://registry.npmjs.org/is-network-error/-/is-network-error-1.3.2.tgz", + "integrity": "sha512-PhBY86zaxNZUuWP6h13Vu5oFe0XY6/UlKzQnYFELzGVHygP3MxmvTfYSG7GN3aIab/iWudSMgjSnG9Dq+nHrgA==", + "license": "MIT", + "engines": { + "node": ">=16" + }, + "funding": { + "url": "https://github.com/sponsors/sindresorhus" + } + }, "node_modules/is-number": { "version": "7.0.0", "resolved": "https://registry.npmjs.org/is-number/-/is-number-7.0.0.tgz", @@ -1600,18 +1634,6 @@ "node": ">=0.12.0" } }, - "node_modules/is-retry-allowed": { - "version": "2.2.0", - "resolved": "https://registry.npmjs.org/is-retry-allowed/-/is-retry-allowed-2.2.0.tgz", - "integrity": "sha512-XVm7LOeLpTW4jV19QSH38vkswxoLud8sQ57YwJVTPWdiaI9I8keEhGFpBlslyVsgdQy4Opg8QOLb8YRgsyZiQg==", - "license": "MIT", - "engines": { - "node": ">=10" - }, - "funding": { - "url": "https://github.com/sponsors/sindresorhus" - } - }, "node_modules/is-stream": { "version": "2.0.1", "resolved": "https://registry.npmjs.org/is-stream/-/is-stream-2.0.1.tgz", @@ -1872,16 +1894,18 @@ "license": "MIT" }, "node_modules/p-retry": { - "version": "4.6.2", - "resolved": "https://registry.npmjs.org/p-retry/-/p-retry-4.6.2.tgz", - "integrity": "sha512-312Id396EbJdvRONlngUx0NydfrIQ5lsYu0znKVUzVvArzEIt08V1qhtyESbGVd1FGX7UKtiFp5uwKZdM8wIuQ==", + "version": "8.0.0", + "resolved": "https://registry.npmjs.org/p-retry/-/p-retry-8.0.0.tgz", + "integrity": "sha512-kFVqH1HxOHp8LupNsOys7bSV09VYTRLxarH/mokO4Rqhk6wGi70E0jh4VzvVGXfEVNggHoHLAMWsQqHyU1Ey9A==", "license": "MIT", "dependencies": { - "@types/retry": "0.12.0", - "retry": "^0.13.1" + "is-network-error": "^1.3.0" }, "engines": { - "node": ">=8" + "node": ">=22" + }, + "funding": { + "url": "https://github.com/sponsors/sindresorhus" } }, "node_modules/p-timeout": { diff --git a/package.json b/package.json index fd66e2a0..95b21ce2 100644 --- a/package.json +++ b/package.json @@ -46,12 +46,12 @@ "@actions/github": "^9.1.1", "@slack/logger": "^4.0.1", "@slack/web-api": "^7.16.0", - "axios": "^1.16.0", - "axios-retry": "^4.5.0", + "@slack/webhook": "file:../node-slack-sdk/packages/webhook", "flat": "^6.0.1", "https-proxy-agent": "^9.0.0", "js-yaml": "^4.2.0", - "markup-js": "^1.5.21" + "markup-js": "^1.5.21", + "p-retry": "^8.0.0" }, "devDependencies": { "@biomejs/biome": "^2.4.16", From ff62bc47ffcdcfc0f16c0767825a68930404a511 Mon Sep 17 00:00:00 2001 From: Eden Zimbelman Date: Tue, 30 Jun 2026 04:03:33 -0700 Subject: [PATCH 2/8] test: stub @slack/webhook send() instead of axios Co-Authored-By: Claude --- test/index.spec.js | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/test/index.spec.js b/test/index.spec.js index 728102be..3ac95772 100644 --- a/test/index.spec.js +++ b/test/index.spec.js @@ -1,6 +1,7 @@ import fs from "node:fs"; import webapi from "@slack/web-api"; -import axios, { AxiosError } from "axios"; +import { IncomingWebhook, WebhookTrigger } from "@slack/webhook"; +import { AxiosError } from "axios"; import sinon from "sinon"; /** @@ -42,7 +43,8 @@ export class Mock { */ constructor() { this.sandbox = sinon.createSandbox(); - this.axios = this.sandbox.stub(axios); + this.incomingWebhook = this.sandbox.stub(IncomingWebhook.prototype, "send"); + this.webhookTrigger = this.sandbox.stub(WebhookTrigger.prototype, "send"); this.calls = this.sandbox.stub(webapi.WebClient.prototype, "apiCall"); this.core = { debug: this.sandbox.stub(), @@ -73,7 +75,8 @@ export class Mock { */ reset() { this.sandbox.reset(); - this.axios.post.resetHistory(); + this.incomingWebhook.resetHistory(); + this.webhookTrigger.resetHistory(); this.calls.resetHistory(); this.core.debug.reset(); this.core.error.reset(); From b212a6666c79d59b760f70149de44ff5e264925f Mon Sep 17 00:00:00 2001 From: Eden Zimbelman Date: Tue, 30 Jun 2026 04:04:47 -0700 Subject: [PATCH 3/8] refactor: drop axios from config, let SDK own webhook User-Agent Co-Authored-By: Claude --- src/config.js | 16 +++------------- test/config.spec.js | 26 +++++++++++++------------- 2 files changed, 16 insertions(+), 26 deletions(-) diff --git a/src/config.js b/src/config.js index 9febb986..e80ee71f 100644 --- a/src/config.js +++ b/src/config.js @@ -1,6 +1,4 @@ -import os from "node:os"; import webapi from "@slack/web-api"; -import axios from "axios"; import packageJson from "../package.json" with { type: "json" }; import Content from "./content.js"; import SlackError from "./errors.js"; @@ -60,11 +58,6 @@ export default class Config { */ inputs; - /** - * @type {import("axios").AxiosStatic} - The axios client. - */ - axios; - /** * @type {Content} - The parsed payload data to send. */ @@ -98,7 +91,6 @@ export default class Config { * @param {import("@actions/core")} core - GitHub Actions core utilities. */ constructor(core) { - this.axios = axios; this.core = core; this.logger = new Logger(core).logger; this.webapi = webapi; @@ -131,17 +123,15 @@ export default class Config { /** * Add user agent metadata for instrumentation. + * + * The @slack/webhook and @slack/web-api SDKs set their own User-Agent + * headers, so the action only registers app metadata with web-api here. */ instrument() { this.webapi.addAppMetadata({ name: packageJson.name, version: packageJson.version, }); - this.axios.defaults.headers.common["User-Agent"] = - `${packageJson.name.replace("/", ":")}/${packageJson.version} ` + - `axios/${this.axios.VERSION} ` + - `node/${process.version.replace("v", "")} ` + - `${os.platform()}/${os.release()}`; } /** diff --git a/test/config.spec.js b/test/config.spec.js index 17c292f3..a0991e25 100644 --- a/test/config.spec.js +++ b/test/config.spec.js @@ -184,19 +184,19 @@ describe("config", () => { } }); - it("adds metadata to webhook with package name and version", async () => { + it("adds web-api app metadata with package name and version", async () => { mocks.core.getInput.withArgs("method").returns("chat.postMessage"); mocks.core.getInput.withArgs("token").returns("xoxb-example"); - const config = new Config(mocks.core); - assert.ok( - config.axios.defaults.headers.common["User-Agent"].startsWith( - "@slack:slack-github-action/", - ), - ); - assert.ok( - config.axios.defaults.headers.common["User-Agent"].length > - "@slack:slack-github-action/".length, - ); + const addAppMetadata = mocks.sandbox.stub(webapi, "addAppMetadata"); + try { + new Config(mocks.core); + assert.ok(addAppMetadata.called, "addAppMetadata was not called"); + const arg = addAppMetadata.getCall(0).firstArg; + assert.ok(arg.name.includes("slack-github-action")); + assert.ok(arg.version.length > 0); + } finally { + addAppMetadata.restore(); + } }); }); @@ -225,7 +225,7 @@ describe("config", () => { describe("validate", () => { it('allow the "retries" option with lowercased space', async () => { - mocks.axios.post.returns(Promise.resolve("LGTM")); + mocks.incomingWebhook.resolves({ text: "LGTM" }); mocks.core.getInput.withArgs("retries").returns(" rapid "); mocks.core.getInput .withArgs("webhook") @@ -247,7 +247,7 @@ describe("config", () => { }); it("errors if an invalid retries option is provided", async () => { - mocks.axios.post.returns(Promise.resolve("LGTM")); + mocks.incomingWebhook.resolves({ text: "LGTM" }); mocks.core.getInput.withArgs("retries").returns("FOREVER"); mocks.core.getInput .withArgs("webhook") From f6e2eae82c423ea43a63dc2992359a38b438877b Mon Sep 17 00:00:00 2001 From: Eden Zimbelman Date: Tue, 30 Jun 2026 04:06:06 -0700 Subject: [PATCH 4/8] test: webhook techniques via @slack/webhook with p-retry gating Co-Authored-By: Claude --- test/webhook.spec.js | 283 +++++++++++-------------------------------- 1 file changed, 70 insertions(+), 213 deletions(-) diff --git a/test/webhook.spec.js b/test/webhook.spec.js index 09867e68..8a328fd5 100644 --- a/test/webhook.spec.js +++ b/test/webhook.spec.js @@ -1,6 +1,5 @@ import assert from "node:assert"; import { beforeEach, describe, it } from "node:test"; -import { AxiosError } from "axios"; import Config from "../src/config.js"; import SlackError from "../src/errors.js"; import send from "../src/send.js"; @@ -19,27 +18,19 @@ describe("webhook", () => { .returns("https://hooks.slack.com"); mocks.core.getInput.withArgs("webhook-type").returns("webhook-trigger"); mocks.core.getInput.withArgs("payload").returns("drinks: coffee"); - mocks.axios.post.returns( - Promise.resolve({ status: 200, data: { ok: true } }), + mocks.webhookTrigger.resolves({ ok: true, body: { ok: true } }); + await send(mocks.core); + assert.equal(mocks.webhookTrigger.getCalls().length, 1); + assert.deepEqual(mocks.webhookTrigger.getCall(0).firstArg, { + drinks: "coffee", + }); + assert.equal(mocks.core.setOutput.getCall(0).firstArg, "ok"); + assert.equal(mocks.core.setOutput.getCall(0).lastArg, true); + assert.equal(mocks.core.setOutput.getCall(1).firstArg, "response"); + assert.equal( + mocks.core.setOutput.getCall(1).lastArg, + JSON.stringify({ ok: true }), ); - try { - await send(mocks.core); - assert.equal(mocks.axios.post.getCalls().length, 1); - const [url, payload, options] = mocks.axios.post.getCall(0).args; - assert.equal(url, "https://hooks.slack.com"); - assert.deepEqual(payload, { drinks: "coffee" }); - assert.deepEqual(options, {}); - assert.equal(mocks.core.setOutput.getCall(0).firstArg, "ok"); - assert.equal(mocks.core.setOutput.getCall(0).lastArg, true); - assert.equal(mocks.core.setOutput.getCall(1).firstArg, "response"); - assert.equal( - mocks.core.setOutput.getCall(1).lastArg, - JSON.stringify({ ok: true }), - ); - } catch (err) { - console.error(err); - assert.fail("Failed to send the webhook"); - } }); it("sends the parsed payload to the provided incoming webhook", async () => { @@ -48,25 +39,19 @@ describe("webhook", () => { .returns("https://hooks.slack.com"); mocks.core.getInput.withArgs("webhook-type").returns("incoming-webhook"); mocks.core.getInput.withArgs("payload").returns("text: greetings"); - mocks.axios.post.returns(Promise.resolve({ status: 200, data: "ok" })); - try { - await send(mocks.core); - assert.equal(mocks.axios.post.getCalls().length, 1); - const [url, payload, options] = mocks.axios.post.getCall(0).args; - assert.equal(url, "https://hooks.slack.com"); - assert.deepEqual(payload, { text: "greetings" }); - assert.deepEqual(options, {}); - assert.equal(mocks.core.setOutput.getCall(0).firstArg, "ok"); - assert.equal(mocks.core.setOutput.getCall(0).lastArg, true); - assert.equal(mocks.core.setOutput.getCall(1).firstArg, "response"); - assert.equal( - mocks.core.setOutput.getCall(1).lastArg, - JSON.stringify("ok"), - ); - } catch (err) { - console.error(err); - assert.fail("Failed to send the webhook"); - } + mocks.incomingWebhook.resolves({ text: "ok" }); + await send(mocks.core); + assert.equal(mocks.incomingWebhook.getCalls().length, 1); + assert.deepEqual(mocks.incomingWebhook.getCall(0).firstArg, { + text: "greetings", + }); + assert.equal(mocks.core.setOutput.getCall(0).firstArg, "ok"); + assert.equal(mocks.core.setOutput.getCall(0).lastArg, true); + assert.equal(mocks.core.setOutput.getCall(1).firstArg, "response"); + assert.equal( + mocks.core.setOutput.getCall(1).lastArg, + JSON.stringify("ok"), + ); }); }); @@ -91,126 +76,68 @@ describe("webhook", () => { } }); - it("returns the failures from a webhook trigger", async () => { + it("does not retry a 4xx from a webhook trigger", async () => { mocks.core.getInput .withArgs("webhook") .returns("https://hooks.slack.com"); mocks.core.getInput.withArgs("webhook-type").returns("webhook-trigger"); mocks.core.getInput.withArgs("payload").returns("drinks: coffee"); - const response = new AxiosError( - "Request failed with status code 400", - "ERR_BAD_REQUEST", - {}, - {}, - { status: 400 }, - ); - mocks.axios.post.resolves(Promise.reject(response)); + mocks.core.getInput.withArgs("retries").returns("RAPID"); + const err = Object.assign(new Error("An HTTP protocol error occurred"), { + code: "slack_webhook_http_error", + original: { response: { status: 400 } }, + }); + mocks.webhookTrigger.rejects(err); try { await send(mocks.core); - } catch (err) { - if (err instanceof SlackError) { - assert.ok( - err.message.includes("Request failed with status code 400"), - ); - } else { - assert.fail(err); - } + } catch (e) { + assert.ok(e instanceof SlackError); } - assert.equal(mocks.axios.post.getCalls().length, 1); - const [url, payload, options] = mocks.axios.post.getCall(0).args; - assert.equal(url, "https://hooks.slack.com"); - assert.deepEqual(payload, { drinks: "coffee" }); - assert.deepEqual(options, {}); + assert.equal( + mocks.webhookTrigger.getCalls().length, + 1, + "4xx must not be retried", + ); assert.equal(mocks.core.setOutput.getCall(0).firstArg, "ok"); assert.equal(mocks.core.setOutput.getCall(0).lastArg, false); - assert.equal(mocks.core.setOutput.getCall(1).firstArg, "response"); }); - it("returns the failures from an incoming webhook", async () => { + it("retries a 5xx from an incoming webhook then succeeds", async () => { mocks.core.getInput .withArgs("webhook") .returns("https://hooks.slack.com"); mocks.core.getInput.withArgs("webhook-type").returns("incoming-webhook"); - mocks.core.getInput.withArgs("payload").returns("textt: oops"); - const response = new AxiosError( - "Request failed with status code 400", - "ERR_BAD_REQUEST", - {}, - {}, - { status: 400 }, + mocks.core.getInput.withArgs("payload").returns("text: hi"); + mocks.core.getInput.withArgs("retries").returns("RAPID"); + const err = Object.assign(new Error("An HTTP protocol error occurred"), { + code: "slack_webhook_http_error", + original: { response: { status: 503 } }, + }); + mocks.incomingWebhook.onFirstCall().rejects(err); + mocks.incomingWebhook.onSecondCall().resolves({ text: "ok" }); + await send(mocks.core); + assert.equal( + mocks.incomingWebhook.getCalls().length, + 2, + "5xx should be retried once", ); - mocks.axios.post.resolves(Promise.reject(response)); - try { - await send(mocks.core); - } catch (err) { - if (err instanceof SlackError) { - assert.ok( - err.message.includes("Request failed with status code 400"), - ); - } else { - assert.fail(err); - } - } - assert.equal(mocks.axios.post.getCalls().length, 1); - const [url, payload, options] = mocks.axios.post.getCall(0).args; - assert.equal(url, "https://hooks.slack.com"); - assert.deepEqual(payload, { textt: "oops" }); - assert.deepEqual(options, {}); - assert.equal(mocks.core.setOutput.getCall(0).firstArg, "ok"); - assert.equal(mocks.core.setOutput.getCall(0).lastArg, false); - assert.equal(mocks.core.setOutput.getCall(1).firstArg, "response"); + assert.equal(mocks.core.setOutput.getCall(0).lastArg, true); }); }); describe("proxies", () => { - it("requires a webhook is included in the inputs", async () => { - /** - * @type {Config} - */ - const config = { - core: mocks.core, - inputs: {}, - }; - try { - new Webhook().proxies(config); - assert.fail("Failed to throw for missing input"); - } catch (err) { - if (err instanceof SlackError) { - assert.ok( - err.message.includes("No webhook was provided to proxy to"), - ); - } else { - assert.fail(err); - } - } - }); - - it("skips proxying an http webhook url altogether", async () => { - mocks.core.getInput.withArgs("webhook").returns("http://hooks.slack.com"); - mocks.core.getInput.withArgs("webhook-type").returns("incoming-webhook"); - mocks.core.getInput.withArgs("proxy").returns("https://example.com"); - const config = new Config(mocks.core); - const webhook = new Webhook(); - const request = webhook.proxies(config); - assert.strictEqual(request, undefined); - }); - - it("sets up the proxy agent for the provided https proxy", async () => { - const proxy = "https://example.com"; + it("returns undefined when no proxy is set", async () => { mocks.core.getInput .withArgs("webhook") .returns("https://hooks.slack.com"); mocks.core.getInput.withArgs("webhook-type").returns("incoming-webhook"); - mocks.core.getInput.withArgs("proxy").returns(proxy); const config = new Config(mocks.core); const webhook = new Webhook(); - const { httpsAgent, proxy: proxying } = webhook.proxies(config); - assert.deepEqual(httpsAgent.proxy, new URL(proxy)); - assert.notStrictEqual(proxying, false); + assert.strictEqual(webhook.proxies(config), undefined); }); - it("sets up the agent without proxy for http proxies", async () => { - const proxy = "http://example.com"; + it("sets up the proxy agent for the provided https proxy", async () => { + const proxy = "https://example.com"; mocks.core.getInput .withArgs("webhook") .returns("https://hooks.slack.com"); @@ -218,9 +145,8 @@ describe("webhook", () => { mocks.core.getInput.withArgs("proxy").returns(proxy); const config = new Config(mocks.core); const webhook = new Webhook(); - const { httpsAgent, proxy: proxying } = webhook.proxies(config); + const { httpsAgent } = webhook.proxies(config); assert.deepEqual(httpsAgent.proxy, new URL(proxy)); - assert.strictEqual(proxying, false); }); it("fails to configure proxies with an invalid proxied url", async () => { @@ -245,102 +171,33 @@ describe("webhook", () => { } } }); - - it("fails to configure proxies with an unknown url protocol", async () => { - const proxy = "ssh://"; - mocks.core.getInput - .withArgs("webhook") - .returns("https://hooks.slack.com"); - mocks.core.getInput.withArgs("webhook-type").returns("incoming-webhook"); - mocks.core.getInput.withArgs("proxy").returns(proxy); - try { - const config = new Config(mocks.core); - const webhook = new Webhook(); - webhook.proxies(config); - assert.fail("An unknown URL protocol was not thrown as error!"); - } catch (err) { - if (err instanceof SlackError) { - assert.ok( - err.message.includes("Failed to configure the HTTPS proxy"), - ); - assert.ok(err.cause.message.includes("Unsupported URL protocol")); - } else { - assert.fail(err); - } - } - }); }); describe("retries", () => { - it("uses a default of five retries in requests", async () => { - const webhook = new Webhook(); - const result = webhook.retries(); + it("uses a default of five retries", async () => { + const result = new Webhook().retries(); assert.equal(result.retries, 5); }); it('does not attempt retries when "0" is set', async () => { - const webhook = new Webhook(); - const result = webhook.retries("0"); + const result = new Webhook().retries("0"); assert.equal(result.retries, 0); }); - it('attempts a default amount of "5" retries', async () => { - const webhook = new Webhook(); - const result = webhook.retries("5"); + it('maps "5" to a five-retry policy', async () => { + const result = new Webhook().retries("5"); assert.equal(result.retries, 5); - if (!result.retryDelay) { - assert.fail("No retry delay found!"); - } - assert.equal( - result.retryDelay(5, mocks.errors.axios.network_failed), - 300000, - "5th retry after 5 seconds", - ); }); - it('attempts "10" retries in around "30" minutes', async () => { - const webhook = new Webhook(); - const result = webhook.retries("10"); + it('maps "10" to a ten-retry policy', async () => { + const result = new Webhook().retries("10"); assert.equal(result.retries, 10); - if (!result.retryDelay) { - assert.fail("No retry delay found!"); - } - assert.ok( - result.retryDelay(10, mocks.errors.axios.network_failed) > 1800000, - "last attempt is around 30 minutes after starting", - ); - assert.ok( - result.retryDelay(10, mocks.errors.axios.network_failed) < 3600000, - "last attempt is no more than an hour later", - ); }); - it('attempts a " rapid" burst of "12" retries in seconds', async () => { - const webhook = new Webhook(); - const result = webhook.retries(" rapid"); - assert.equal(result.retries, 12); - if (!result.retryDelay) { - assert.fail("No retry delay found!"); - } - assert.equal( - result.retryDelay(12, mocks.errors.axios.network_failed), - 12000, - "12th retry after 12 seconds", - ); - }); - - it('attempts a "RAPID" burst of "12" retries in seconds', async () => { - const webhook = new Webhook(); - const result = webhook.retries("RAPID"); - assert.equal(result.retries, 12); - if (!result.retryDelay) { - assert.fail("No retry delay found!"); - } - assert.equal( - result.retryDelay(12, mocks.errors.axios.network_failed), - 12000, - "12th retry after 12 seconds", - ); + it('maps "RAPID" (case/space-insensitive) to a rapid policy', async () => { + const rapid = new Webhook().retries("RAPID"); + const spaced = new Webhook().retries(" rapid"); + assert.deepEqual(rapid, spaced); }); }); }); From 074e58fa6f80b9a26f7cf1249cfe7a93e5a94982 Mon Sep 17 00:00:00 2001 From: Eden Zimbelman Date: Tue, 30 Jun 2026 04:08:39 -0700 Subject: [PATCH 5/8] feat: post webhooks via @slack/webhook with action-side retries Co-Authored-By: Claude --- src/webhook.js | 190 +++++++++++++++++++++++++++----------------- test/config.spec.js | 14 ---- test/index.spec.js | 16 ---- test/send.spec.js | 6 +- 4 files changed, 117 insertions(+), 109 deletions(-) diff --git a/src/webhook.js b/src/webhook.js index 0d6df7d4..91226491 100644 --- a/src/webhook.js +++ b/src/webhook.js @@ -1,11 +1,15 @@ -import axiosRetry, { exponentialDelay, linearDelay } from "axios-retry"; +import webapi from "@slack/web-api"; +import { ErrorCode, IncomingWebhook, WebhookTrigger } from "@slack/webhook"; import { HttpsProxyAgent } from "https-proxy-agent"; +import pRetry, { AbortError } from "p-retry"; import Config from "./config.js"; import SlackError from "./errors.js"; /** - * This Webhook class posts the configured payload to the provided webhook, with - * whatever additional settings set. + * The Webhook class posts the configured payload to the provided webhook using + * the @slack/webhook SDK, choosing the client by the configured webhook type. + * + * @see {@link https://docs.slack.dev/tools/node-slack-sdk/webhook/} */ export default class Webhook { /** @@ -15,69 +19,121 @@ export default class Webhook { if (!config.inputs.webhook) { throw new SlackError(config.core, "No webhook was provided to post to"); } - /** - * @type {import("axios-retry").IAxiosRetryConfig} - * @see {@link https://www.npmjs.com/package/axios-retry} - */ - const retries = this.retries(config.inputs.retries); - axiosRetry(config.axios, retries); + switch (config.inputs.webhookType) { + case "incoming-webhook": + return await this.postIncomingWebhook(config); + case "webhook-trigger": + return await this.postWebhookTrigger(config); + default: + throw new SlackError( + config.core, + `Unknown webhook type: ${config.inputs.webhookType}`, + ); + } + } + + /** + * Post using the @slack/webhook IncomingWebhook client. + * @param {Config} config + */ + async postIncomingWebhook(config) { + const webhook = new IncomingWebhook( + /** @type {string} */ (config.inputs.webhook), + { agent: this.proxies(config)?.httpsAgent }, + ); try { - const response = await config.axios.post( - config.inputs.webhook, - config.content.values, - { - ...this.proxies(config), - }, + const response = await this.send(config, () => + webhook.send(config.content.values), ); - config.core.setOutput("ok", response.status === 200); - config.core.setOutput("response", JSON.stringify(response.data)); - config.core.debug(JSON.stringify(response.data)); + config.core.setOutput("ok", true); + config.core.setOutput("response", JSON.stringify(response.text)); + config.core.debug(JSON.stringify(response.text)); } catch (/** @type {any} */ err) { - const response = err.toJSON(); - config.core.setOutput("ok", response.status === 200); - config.core.setOutput("response", JSON.stringify(response.message)); - config.core.debug(response); - throw new SlackError(config.core, response.message); + config.core.setOutput("ok", false); + config.core.setOutput("response", JSON.stringify(err.message)); + config.core.debug(err); + throw new SlackError(config.core, err.message); } } /** - * Return configurations for http proxy options if these are set. + * Post using the @slack/webhook WebhookTrigger client. * @param {Config} config - * @returns {import("axios").AxiosRequestConfig | undefined} - * @see {@link https://github.com/slackapi/slack-github-action/pull/132} */ - proxies(config) { - const { webhook, proxy } = config.inputs; - if (!webhook) { - throw new SlackError(config.core, "No webhook was provided to proxy to"); + async postWebhookTrigger(config) { + const trigger = new WebhookTrigger( + /** @type {string} */ (config.inputs.webhook), + { agent: this.proxies(config)?.httpsAgent }, + ); + try { + const response = await this.send(config, () => + trigger.send(config.content.values), + ); + config.core.setOutput("ok", response.ok); + config.core.setOutput("response", JSON.stringify(response.body)); + config.core.debug(JSON.stringify(response.body)); + } catch (/** @type {any} */ err) { + config.core.setOutput("ok", false); + config.core.setOutput("response", JSON.stringify(err.message)); + config.core.debug(err); + throw new SlackError(config.core, err.message); } - if (!proxy) { - return undefined; + } + + /** + * Invoke a webhook send with retries, aborting on non-retryable errors. + * @template T + * @param {Config} config + * @param {() => Promise} attempt - the SDK send call to retry. + * @returns {Promise} + */ + async send(config, attempt) { + return await pRetry(async () => { + try { + return await attempt(); + } catch (/** @type {any} */ err) { + if (this.retryable(err)) { + throw err; + } + throw new AbortError(err); + } + }, this.retries(config.inputs.retries)); + } + + /** + * Decide if a @slack/webhook error should be retried. + * + * Request errors (no response received) are always retried; HTTP errors are + * retried only for rate limits and server errors. + * @param {any} err + * @returns {boolean} + */ + retryable(err) { + if (err?.code === ErrorCode.RequestError) { + return true; } + if (err?.code === ErrorCode.HTTPError) { + const status = err?.original?.response?.status; + return status === 429 || (status >= 500 && status <= 599); + } + return true; + } + + /** + * Return configurations for https proxy options if these are set. + * @param {Config} config + * @returns {{ httpsAgent: HttpsProxyAgent } | undefined} + * @see {@link https://github.com/slackapi/slack-github-action/pull/205} + */ + proxies(config) { + const proxy = config.inputs.proxy; try { - if (new URL(webhook).protocol !== "https:") { - config.core.debug( - "The webhook destination is not HTTPS so skipping the HTTPS proxy", - ); + if (!proxy) { return undefined; } - switch (new URL(proxy).protocol) { - case "https:": - return { - httpsAgent: new HttpsProxyAgent(proxy), - }; - case "http:": - return { - httpsAgent: new HttpsProxyAgent(proxy), - proxy: false, - }; - default: - throw new SlackError( - config.core, - `Unsupported URL protocol: ${proxy}`, - ); - } + return { + httpsAgent: new HttpsProxyAgent(proxy), + }; } catch (/** @type {any} */ err) { throw new SlackError(config.core, "Failed to configure the HTTPS proxy", { cause: err, @@ -86,38 +142,22 @@ export default class Webhook { } /** - * Return configurations for retry options with different delays. - * @param {string} option - * @returns {import("axios-retry").IAxiosRetryConfig} + * Map the retries input to a p-retry / node-retry policy. + * @param {string} [option] + * @returns {import("@slack/web-api").RetryOptions} */ retries(option) { switch (option?.trim().toUpperCase()) { case "0": return { retries: 0 }; case "5": - return { - retryCondition: axiosRetry.isRetryableError, - retries: 5, - retryDelay: linearDelay(60 * 1000), // 5 minutes - }; + return webapi.retryPolicies.fiveRetriesInFiveMinutes; case "10": - return { - retryCondition: axiosRetry.isRetryableError, - retries: 10, - retryDelay: (count, err) => exponentialDelay(count, err, 2 * 1000), // 34.12 minutes - }; + return webapi.retryPolicies.tenRetriesInAboutThirtyMinutes; case "RAPID": - return { - retryCondition: axiosRetry.isRetryableError, - retries: 12, - retryDelay: linearDelay(1 * 1000), // 12 seconds - }; + return webapi.retryPolicies.rapidRetryPolicy; default: - return { - retryCondition: axiosRetry.isRetryableError, - retries: 5, - retryDelay: linearDelay(60 * 1000), // 5 minutes - }; + return webapi.retryPolicies.fiveRetriesInFiveMinutes; } } } diff --git a/test/config.spec.js b/test/config.spec.js index a0991e25..c582e514 100644 --- a/test/config.spec.js +++ b/test/config.spec.js @@ -184,20 +184,6 @@ describe("config", () => { } }); - it("adds web-api app metadata with package name and version", async () => { - mocks.core.getInput.withArgs("method").returns("chat.postMessage"); - mocks.core.getInput.withArgs("token").returns("xoxb-example"); - const addAppMetadata = mocks.sandbox.stub(webapi, "addAppMetadata"); - try { - new Config(mocks.core); - assert.ok(addAppMetadata.called, "addAppMetadata was not called"); - const arg = addAppMetadata.getCall(0).firstArg; - assert.ok(arg.name.includes("slack-github-action")); - assert.ok(arg.version.length > 0); - } finally { - addAppMetadata.restore(); - } - }); }); describe("mask", async () => { diff --git a/test/index.spec.js b/test/index.spec.js index 3ac95772..929af4ca 100644 --- a/test/index.spec.js +++ b/test/index.spec.js @@ -1,7 +1,6 @@ import fs from "node:fs"; import webapi from "@slack/web-api"; import { IncomingWebhook, WebhookTrigger } from "@slack/webhook"; -import { AxiosError } from "axios"; import sinon from "sinon"; /** @@ -20,21 +19,6 @@ import sinon from "sinon"; * The Mock class sets expected behaviors and test listeners for dependencies. */ export class Mock { - /** - * @typedef Errors - A collection of mocked errors to use in tests. - * @prop {Object.} axios - The mocked axios errors. - */ - - /** - * The mocked errors. - * @type {Errors} - */ - errors = { - axios: { - network_failed: new AxiosError("network_failed"), - }, - }; - /** * Setup stubbed dependencies and configure default input arguments for all * tests. diff --git a/test/send.spec.js b/test/send.spec.js index 9ab906ee..eef7111d 100644 --- a/test/send.spec.js +++ b/test/send.spec.js @@ -24,9 +24,7 @@ describe("send", () => { .returns("https://hooks.slack.com"); mocks.core.getInput.withArgs("webhook-type").returns("webhook-trigger"); mocks.core.getInput.withArgs("payload").returns('"greetings": "hello"'); - mocks.axios.post.returns( - Promise.resolve({ status: 200, data: { ok: true } }), - ); + mocks.webhookTrigger.resolves({ ok: true, body: { ok: true } }); await send(mocks.core); assert.equal(mocks.core.setOutput.getCall(0).firstArg, "ok"); assert.equal(mocks.core.setOutput.getCall(0).lastArg, true); @@ -64,7 +62,7 @@ describe("send", () => { .returns("https://hooks.slack.com"); mocks.core.getInput.withArgs("webhook-type").returns("incoming-webhook"); mocks.core.getInput.withArgs("payload").returns('"text": "hello"'); - mocks.axios.post.returns(Promise.resolve({ status: 200, data: "ok" })); + mocks.incomingWebhook.resolves({ text: "ok" }); await send(mocks.core); assert.equal(mocks.core.setOutput.getCall(0).firstArg, "ok"); assert.equal(mocks.core.setOutput.getCall(0).lastArg, true); From 12111a6862a683811c10888e203544422b643747 Mon Sep 17 00:00:00 2001 From: Eden Zimbelman Date: Tue, 30 Jun 2026 04:09:08 -0700 Subject: [PATCH 6/8] test: drop trailing blank after removing duplicate metadata test Co-Authored-By: Claude --- test/config.spec.js | 1 - 1 file changed, 1 deletion(-) diff --git a/test/config.spec.js b/test/config.spec.js index c582e514..b51310b1 100644 --- a/test/config.spec.js +++ b/test/config.spec.js @@ -183,7 +183,6 @@ describe("config", () => { Object.defineProperty(webapi, "addAppMetadata", original); } }); - }); describe("mask", async () => { From f0480dfd5442a049e896f139b04a151271e0ccfb Mon Sep 17 00:00:00 2001 From: Eden Zimbelman Date: Thu, 2 Jul 2026 01:53:30 -0700 Subject: [PATCH 7/8] refactor: delegate webhook retries to @slack/webhook SDK Retries now live in the @slack/webhook package via retryConfig, matching how the API-method technique passes retryConfig to @slack/web-api. The action maps its 0/5/10/RAPID input to the SDK's exported retry policies. Co-Authored-By: Claude --- src/webhook.js | 82 ++++++++++++++------------------------------ test/webhook.spec.js | 42 +++++++++-------------- 2 files changed, 42 insertions(+), 82 deletions(-) diff --git a/src/webhook.js b/src/webhook.js index 91226491..b1ca916c 100644 --- a/src/webhook.js +++ b/src/webhook.js @@ -1,7 +1,11 @@ -import webapi from "@slack/web-api"; -import { ErrorCode, IncomingWebhook, WebhookTrigger } from "@slack/webhook"; +import { + fiveRetriesInFiveMinutes, + IncomingWebhook, + rapidRetryPolicy, + tenRetriesInAboutThirtyMinutes, + WebhookTrigger, +} from "@slack/webhook"; import { HttpsProxyAgent } from "https-proxy-agent"; -import pRetry, { AbortError } from "p-retry"; import Config from "./config.js"; import SlackError from "./errors.js"; @@ -9,6 +13,9 @@ import SlackError from "./errors.js"; * The Webhook class posts the configured payload to the provided webhook using * the @slack/webhook SDK, choosing the client by the configured webhook type. * + * Retries are delegated to the SDK via the retryConfig option, matching how + * the API-method technique passes retryConfig to @slack/web-api's WebClient. + * * @see {@link https://docs.slack.dev/tools/node-slack-sdk/webhook/} */ export default class Webhook { @@ -39,12 +46,13 @@ export default class Webhook { async postIncomingWebhook(config) { const webhook = new IncomingWebhook( /** @type {string} */ (config.inputs.webhook), - { agent: this.proxies(config)?.httpsAgent }, + { + agent: this.proxies(config)?.httpsAgent, + retryConfig: this.retries(config.inputs.retries), + }, ); try { - const response = await this.send(config, () => - webhook.send(config.content.values), - ); + const response = await webhook.send(config.content.values); config.core.setOutput("ok", true); config.core.setOutput("response", JSON.stringify(response.text)); config.core.debug(JSON.stringify(response.text)); @@ -63,12 +71,13 @@ export default class Webhook { async postWebhookTrigger(config) { const trigger = new WebhookTrigger( /** @type {string} */ (config.inputs.webhook), - { agent: this.proxies(config)?.httpsAgent }, + { + agent: this.proxies(config)?.httpsAgent, + retryConfig: this.retries(config.inputs.retries), + }, ); try { - const response = await this.send(config, () => - trigger.send(config.content.values), - ); + const response = await trigger.send(config.content.values); config.core.setOutput("ok", response.ok); config.core.setOutput("response", JSON.stringify(response.body)); config.core.debug(JSON.stringify(response.body)); @@ -80,45 +89,6 @@ export default class Webhook { } } - /** - * Invoke a webhook send with retries, aborting on non-retryable errors. - * @template T - * @param {Config} config - * @param {() => Promise} attempt - the SDK send call to retry. - * @returns {Promise} - */ - async send(config, attempt) { - return await pRetry(async () => { - try { - return await attempt(); - } catch (/** @type {any} */ err) { - if (this.retryable(err)) { - throw err; - } - throw new AbortError(err); - } - }, this.retries(config.inputs.retries)); - } - - /** - * Decide if a @slack/webhook error should be retried. - * - * Request errors (no response received) are always retried; HTTP errors are - * retried only for rate limits and server errors. - * @param {any} err - * @returns {boolean} - */ - retryable(err) { - if (err?.code === ErrorCode.RequestError) { - return true; - } - if (err?.code === ErrorCode.HTTPError) { - const status = err?.original?.response?.status; - return status === 429 || (status >= 500 && status <= 599); - } - return true; - } - /** * Return configurations for https proxy options if these are set. * @param {Config} config @@ -142,22 +112,22 @@ export default class Webhook { } /** - * Map the retries input to a p-retry / node-retry policy. + * Map the retries input to a @slack/webhook retry policy. * @param {string} [option] - * @returns {import("@slack/web-api").RetryOptions} + * @returns {import("@slack/webhook").RetryOptions} */ retries(option) { switch (option?.trim().toUpperCase()) { case "0": return { retries: 0 }; case "5": - return webapi.retryPolicies.fiveRetriesInFiveMinutes; + return fiveRetriesInFiveMinutes; case "10": - return webapi.retryPolicies.tenRetriesInAboutThirtyMinutes; + return tenRetriesInAboutThirtyMinutes; case "RAPID": - return webapi.retryPolicies.rapidRetryPolicy; + return rapidRetryPolicy; default: - return webapi.retryPolicies.fiveRetriesInFiveMinutes; + return fiveRetriesInFiveMinutes; } } } diff --git a/test/webhook.spec.js b/test/webhook.spec.js index 8a328fd5..80832b74 100644 --- a/test/webhook.spec.js +++ b/test/webhook.spec.js @@ -76,52 +76,42 @@ describe("webhook", () => { } }); - it("does not retry a 4xx from a webhook trigger", async () => { + it("returns the failures from a webhook trigger", async () => { mocks.core.getInput .withArgs("webhook") .returns("https://hooks.slack.com"); mocks.core.getInput.withArgs("webhook-type").returns("webhook-trigger"); mocks.core.getInput.withArgs("payload").returns("drinks: coffee"); - mocks.core.getInput.withArgs("retries").returns("RAPID"); - const err = Object.assign(new Error("An HTTP protocol error occurred"), { - code: "slack_webhook_http_error", - original: { response: { status: 400 } }, - }); - mocks.webhookTrigger.rejects(err); + mocks.webhookTrigger.rejects( + new Error("An HTTP protocol error occurred"), + ); try { await send(mocks.core); } catch (e) { assert.ok(e instanceof SlackError); } - assert.equal( - mocks.webhookTrigger.getCalls().length, - 1, - "4xx must not be retried", - ); + assert.equal(mocks.webhookTrigger.getCalls().length, 1); assert.equal(mocks.core.setOutput.getCall(0).firstArg, "ok"); assert.equal(mocks.core.setOutput.getCall(0).lastArg, false); }); - it("retries a 5xx from an incoming webhook then succeeds", async () => { + it("returns the failures from an incoming webhook", async () => { mocks.core.getInput .withArgs("webhook") .returns("https://hooks.slack.com"); mocks.core.getInput.withArgs("webhook-type").returns("incoming-webhook"); mocks.core.getInput.withArgs("payload").returns("text: hi"); - mocks.core.getInput.withArgs("retries").returns("RAPID"); - const err = Object.assign(new Error("An HTTP protocol error occurred"), { - code: "slack_webhook_http_error", - original: { response: { status: 503 } }, - }); - mocks.incomingWebhook.onFirstCall().rejects(err); - mocks.incomingWebhook.onSecondCall().resolves({ text: "ok" }); - await send(mocks.core); - assert.equal( - mocks.incomingWebhook.getCalls().length, - 2, - "5xx should be retried once", + mocks.incomingWebhook.rejects( + new Error("An HTTP protocol error occurred"), ); - assert.equal(mocks.core.setOutput.getCall(0).lastArg, true); + try { + await send(mocks.core); + } catch (e) { + assert.ok(e instanceof SlackError); + } + assert.equal(mocks.incomingWebhook.getCalls().length, 1); + assert.equal(mocks.core.setOutput.getCall(0).firstArg, "ok"); + assert.equal(mocks.core.setOutput.getCall(0).lastArg, false); }); }); From 08d6c7a088c99c39035a63b6a9f9ca54e76b5af7 Mon Sep 17 00:00:00 2001 From: Eden Zimbelman Date: Thu, 2 Jul 2026 01:54:11 -0700 Subject: [PATCH 8/8] build(deps): sync lockfile with @slack/webhook retry deps Co-Authored-By: Claude --- package-lock.json | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/package-lock.json b/package-lock.json index daac52a3..c2f5cfe6 100644 --- a/package-lock.json +++ b/package-lock.json @@ -44,7 +44,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"