From 4a2e1845c497f912df5515698eb5ec550cd78597 Mon Sep 17 00:00:00 2001 From: Eden Zimbelman Date: Tue, 4 Aug 2026 15:04:21 -0700 Subject: [PATCH 1/4] feat(webhook): warn when URL pattern mismatches webhook class Add a logger to @slack/webhook that warns at construction time when: - IncomingWebhook receives a URL containing "/triggers/" - WebhookTrigger receives a URL containing "/services/" This helps catch misconfiguration that causes duplicate deliveries or hangs (see #654). Both classes now accept optional `logger` and `logLevel` options via their defaults/options argument. Refs: #654 Co-Authored-By: Claude --- .changeset/webhook-url-mismatch-warning.md | 7 +++++++ package-lock.json | 5 +++-- packages/webhook/package.json | 1 + packages/webhook/src/IncomingWebhook.ts | 23 +++++++++++++++++++++- packages/webhook/src/WebhookTrigger.ts | 14 +++++++++++++ packages/webhook/src/index.ts | 2 ++ packages/webhook/src/logger.ts | 18 +++++++++++++++++ 7 files changed, 67 insertions(+), 3 deletions(-) create mode 100644 .changeset/webhook-url-mismatch-warning.md create mode 100644 packages/webhook/src/logger.ts diff --git a/.changeset/webhook-url-mismatch-warning.md b/.changeset/webhook-url-mismatch-warning.md new file mode 100644 index 000000000..56df58305 --- /dev/null +++ b/.changeset/webhook-url-mismatch-warning.md @@ -0,0 +1,7 @@ +--- +"@slack/webhook": minor +--- + +feat: add logger with URL mismatch warning + +Both `IncomingWebhook` and `WebhookTrigger` now accept optional `logger` and `logLevel` options. On construction, each class warns if the provided URL appears to be intended for the other class (e.g. a `/triggers/` URL passed to `IncomingWebhook`), helping catch misconfiguration before it causes duplicate deliveries or hangs. diff --git a/package-lock.json b/package-lock.json index 5377de0bd..9d4da1ba4 100644 --- a/package-lock.json +++ b/package-lock.json @@ -5587,9 +5587,9 @@ "ws": "^8" }, "devDependencies": { - "@types/sinon": "^22.0.0", + "@types/sinon": "^22", "@types/ws": "^8", - "sinon": "^22.1.0" + "sinon": "^22" }, "engines": { "node": ">=18", @@ -5809,6 +5809,7 @@ "version": "8.0.0", "license": "MIT", "dependencies": { + "@slack/logger": ">=4.0.0", "@slack/types": "^3.0.0", "@types/node": ">=20", "@types/retry": "0.12.5", diff --git a/packages/webhook/package.json b/packages/webhook/package.json index 1dac5777c..6f49db917 100644 --- a/packages/webhook/package.json +++ b/packages/webhook/package.json @@ -41,6 +41,7 @@ "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/logger": ">=4.0.0", "@slack/types": "^3.0.0", "@types/node": ">=20", "@types/retry": "0.12.5", diff --git a/packages/webhook/src/IncomingWebhook.ts b/packages/webhook/src/IncomingWebhook.ts index ba19ee2f6..2fd3b5958 100644 --- a/packages/webhook/src/IncomingWebhook.ts +++ b/packages/webhook/src/IncomingWebhook.ts @@ -3,6 +3,7 @@ import pRetry, { AbortError } from 'p-retry'; import { IncomingWebhookHTTPError, IncomingWebhookRequestError, SlackWebhookError } from './errors'; import { getUserAgent } from './instrument'; +import { getLogger, type Logger, LogLevel } from './logger'; import type { RetryOptions } from './retry-policies'; export interface FetchHeaders { @@ -65,6 +66,8 @@ export class IncomingWebhook { */ private retryConfig: RetryOptions; + private logger: Logger; + public constructor( url: string, defaults: IncomingWebhookDefaultArguments = { @@ -75,6 +78,8 @@ export class IncomingWebhook { throw new Error('Incoming webhook URL is required'); } + this.logger = getLogger('IncomingWebhook', defaults.logLevel ?? LogLevel.INFO, defaults.logger); + this.url = url; this.fetchFn = defaults.fetch ?? globalThis.fetch; this.timeout = defaults.timeout ?? 0; @@ -83,8 +88,22 @@ export class IncomingWebhook { 'User-Agent': getUserAgent(), }; + if (url.includes('/triggers/')) { + this.logger.warn( + 'This URL looks like a webhook trigger (contains "/triggers/"). ' + + 'Consider using WebhookTrigger instead of IncomingWebhook.', + ); + } + // Remove transport options so they don't leak into payloads - const { fetch: _fetch, timeout: _timeout, retryConfig: _retryConfig, ...messageDefaults } = defaults; + const { + fetch: _fetch, + timeout: _timeout, + retryConfig: _retryConfig, + logger: _logger, + logLevel: _logLevel, + ...messageDefaults + } = defaults; this.defaults = messageDefaults; } @@ -159,6 +178,8 @@ export interface IncomingWebhookDefaultArguments { fetch?: FetchFunction; timeout?: number; retryConfig?: RetryOptions; + logger?: Logger; + logLevel?: LogLevel; } export interface IncomingWebhookSendArguments extends IncomingWebhookDefaultArguments { diff --git a/packages/webhook/src/WebhookTrigger.ts b/packages/webhook/src/WebhookTrigger.ts index a9b64e08e..ed79dc319 100644 --- a/packages/webhook/src/WebhookTrigger.ts +++ b/packages/webhook/src/WebhookTrigger.ts @@ -3,6 +3,7 @@ import pRetry, { AbortError } from 'p-retry'; import { SlackWebhookError, WebhookTriggerHTTPError, WebhookTriggerRequestError } from './errors'; import type { FetchFunction, FetchResponse } from './IncomingWebhook'; import { getUserAgent } from './instrument'; +import { getLogger, type Logger, LogLevel } from './logger'; import type { RetryOptions } from './retry-policies'; /** @@ -35,6 +36,8 @@ export class WebhookTrigger { */ private retryConfig: RetryOptions; + private logger: Logger; + public constructor( url: string, defaults: WebhookTriggerDefaultArguments = { @@ -45,6 +48,8 @@ export class WebhookTrigger { throw new Error('Webhook trigger URL is required'); } + this.logger = getLogger('WebhookTrigger', defaults.logLevel ?? LogLevel.INFO, defaults.logger); + this.url = url; this.fetchFn = defaults.fetch ?? globalThis.fetch; this.timeout = defaults.timeout ?? 0; @@ -52,6 +57,13 @@ export class WebhookTrigger { this.headers = { 'User-Agent': getUserAgent(), }; + + if (url.includes('/services/')) { + this.logger.warn( + 'This URL looks like an incoming webhook (contains "/services/"). ' + + 'Consider using IncomingWebhook instead of WebhookTrigger.', + ); + } } /** @@ -114,6 +126,8 @@ export interface WebhookTriggerDefaultArguments { fetch?: FetchFunction; timeout?: number; retryConfig?: RetryOptions; + logger?: Logger; + logLevel?: LogLevel; } export interface WebhookTriggerSendArguments { diff --git a/packages/webhook/src/index.ts b/packages/webhook/src/index.ts index fccbe0f33..4e7eba589 100644 --- a/packages/webhook/src/index.ts +++ b/packages/webhook/src/index.ts @@ -22,6 +22,8 @@ export { export { addAppMetadata } from './instrument'; +export { Logger, LogLevel } from './logger'; + export { default as retryPolicies, RetryOptions } from './retry-policies'; export { diff --git a/packages/webhook/src/logger.ts b/packages/webhook/src/logger.ts new file mode 100644 index 000000000..77346228d --- /dev/null +++ b/packages/webhook/src/logger.ts @@ -0,0 +1,18 @@ +import { ConsoleLogger, type Logger, type LogLevel } from '@slack/logger'; + +export { Logger, LogLevel } from '@slack/logger'; + +let instanceCount = 0; + +export function getLogger(name: string, level: LogLevel, existingLogger?: Logger): Logger { + const instanceId = instanceCount; + instanceCount += 1; + + const logger: Logger = existingLogger ?? new ConsoleLogger(); + logger.setName(`webhook:${name}:${instanceId}`); + if (level !== undefined) { + logger.setLevel(level); + } + + return logger; +} From 3be77a7ffd0dd686ff6bb0b7d91cd24982ed2bc8 Mon Sep 17 00:00:00 2001 From: Eden Zimbelman Date: Tue, 4 Aug 2026 15:08:29 -0700 Subject: [PATCH 2/4] chore(webhook): alphabetize interface fields, use ^5.0.0 for @slack/logger Co-Authored-By: Claude --- package-lock.json | 2 +- packages/webhook/package.json | 2 +- packages/webhook/src/IncomingWebhook.ts | 14 +++++++------- packages/webhook/src/WebhookTrigger.ts | 6 +++--- 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/package-lock.json b/package-lock.json index 9d4da1ba4..8d7a3b3b1 100644 --- a/package-lock.json +++ b/package-lock.json @@ -5809,7 +5809,7 @@ "version": "8.0.0", "license": "MIT", "dependencies": { - "@slack/logger": ">=4.0.0", + "@slack/logger": "^5.0.0", "@slack/types": "^3.0.0", "@types/node": ">=20", "@types/retry": "0.12.5", diff --git a/packages/webhook/package.json b/packages/webhook/package.json index 6f49db917..958fe023e 100644 --- a/packages/webhook/package.json +++ b/packages/webhook/package.json @@ -41,7 +41,7 @@ "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/logger": ">=4.0.0", + "@slack/logger": "^5.0.0", "@slack/types": "^3.0.0", "@types/node": ">=20", "@types/retry": "0.12.5", diff --git a/packages/webhook/src/IncomingWebhook.ts b/packages/webhook/src/IncomingWebhook.ts index 2fd3b5958..f4bed1575 100644 --- a/packages/webhook/src/IncomingWebhook.ts +++ b/packages/webhook/src/IncomingWebhook.ts @@ -169,17 +169,17 @@ export class IncomingWebhook { */ export interface IncomingWebhookDefaultArguments { - username?: string; + channel?: string; + fetch?: FetchFunction; icon_emoji?: string; icon_url?: string; - channel?: string; - text?: string; link_names?: boolean; - fetch?: FetchFunction; - timeout?: number; - retryConfig?: RetryOptions; - logger?: Logger; logLevel?: LogLevel; + logger?: Logger; + retryConfig?: RetryOptions; + text?: string; + timeout?: number; + username?: string; } export interface IncomingWebhookSendArguments extends IncomingWebhookDefaultArguments { diff --git a/packages/webhook/src/WebhookTrigger.ts b/packages/webhook/src/WebhookTrigger.ts index ed79dc319..360ef04fd 100644 --- a/packages/webhook/src/WebhookTrigger.ts +++ b/packages/webhook/src/WebhookTrigger.ts @@ -124,10 +124,10 @@ export class WebhookTrigger { export interface WebhookTriggerDefaultArguments { fetch?: FetchFunction; - timeout?: number; - retryConfig?: RetryOptions; - logger?: Logger; logLevel?: LogLevel; + logger?: Logger; + retryConfig?: RetryOptions; + timeout?: number; } export interface WebhookTriggerSendArguments { From 9484760a4294e1ecba12403acc287e4b155b2297 Mon Sep 17 00:00:00 2001 From: Eden Zimbelman Date: Tue, 4 Aug 2026 15:13:14 -0700 Subject: [PATCH 3/4] chore(webhook): simplify logger helper, clean lockfile Co-Authored-By: Claude --- packages/webhook/src/logger.ts | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/packages/webhook/src/logger.ts b/packages/webhook/src/logger.ts index 77346228d..0e713507c 100644 --- a/packages/webhook/src/logger.ts +++ b/packages/webhook/src/logger.ts @@ -2,17 +2,9 @@ import { ConsoleLogger, type Logger, type LogLevel } from '@slack/logger'; export { Logger, LogLevel } from '@slack/logger'; -let instanceCount = 0; - export function getLogger(name: string, level: LogLevel, existingLogger?: Logger): Logger { - const instanceId = instanceCount; - instanceCount += 1; - const logger: Logger = existingLogger ?? new ConsoleLogger(); - logger.setName(`webhook:${name}:${instanceId}`); - if (level !== undefined) { - logger.setLevel(level); - } - + logger.setName(name); + logger.setLevel(level); return logger; } From d33b86c752a30b2574db1385cfc92a51a4b769ea Mon Sep 17 00:00:00 2001 From: Eden Zimbelman Date: Tue, 4 Aug 2026 15:16:44 -0700 Subject: [PATCH 4/4] test(webhook): add unit tests for URL mismatch warnings Co-Authored-By: Claude --- packages/webhook/src/IncomingWebhook.test.ts | 40 ++++++++++++++++++++ packages/webhook/src/WebhookTrigger.test.ts | 40 ++++++++++++++++++++ 2 files changed, 80 insertions(+) diff --git a/packages/webhook/src/IncomingWebhook.test.ts b/packages/webhook/src/IncomingWebhook.test.ts index 302e94833..3c28e97da 100644 --- a/packages/webhook/src/IncomingWebhook.test.ts +++ b/packages/webhook/src/IncomingWebhook.test.ts @@ -11,6 +11,7 @@ import { } from './errors'; import { type FetchFunction, IncomingWebhook } from './IncomingWebhook'; import { getUserAgent } from './instrument'; +import { LogLevel } from './logger'; import { rapidRetryPolicy } from './retry-policies'; const url = 'https://hooks.slack.com/services/FAKEWEBHOOK'; @@ -49,6 +50,45 @@ describe('IncomingWebhook', () => { await webhook.send('Hello'); assert.ok(fetchCalled); }); + + it('should warn when URL contains /triggers/', () => { + const warnings: string[] = []; + const logger = { + debug() {}, + info() {}, + warn(...msg: string[]) { + warnings.push(msg.join(' ')); + }, + error() {}, + setLevel() {}, + getLevel() { + return LogLevel.WARN; + }, + setName() {}, + }; + new IncomingWebhook('https://hooks.slack.com/triggers/T000/abc', { logger }); + assert.strictEqual(warnings.length, 1); + assert.ok(warnings[0].includes('WebhookTrigger')); + }); + + it('should not warn when URL contains /services/', () => { + const warnings: string[] = []; + const logger = { + debug() {}, + info() {}, + warn(...msg: string[]) { + warnings.push(msg.join(' ')); + }, + error() {}, + setLevel() {}, + getLevel() { + return LogLevel.WARN; + }, + setName() {}, + }; + new IncomingWebhook('https://hooks.slack.com/services/T000/B000/abc', { logger }); + assert.strictEqual(warnings.length, 0); + }); }); describe('send()', () => { diff --git a/packages/webhook/src/WebhookTrigger.test.ts b/packages/webhook/src/WebhookTrigger.test.ts index 303df61eb..80bbdce39 100644 --- a/packages/webhook/src/WebhookTrigger.test.ts +++ b/packages/webhook/src/WebhookTrigger.test.ts @@ -11,6 +11,7 @@ import { WebhookTriggerRequestError, } from './errors'; import { addAppMetadata } from './instrument'; +import { LogLevel } from './logger'; import { rapidRetryPolicy } from './retry-policies'; import { WebhookTrigger } from './WebhookTrigger'; @@ -45,6 +46,45 @@ describe('WebhookTrigger', () => { assert.throws(() => new WebhookTrigger(undefined as any), /URL is required/); assert.throws(() => new WebhookTrigger(''), /URL is required/); }); + + it('should warn when URL contains /services/', () => { + const warnings: string[] = []; + const logger = { + debug() {}, + info() {}, + warn(...msg: string[]) { + warnings.push(msg.join(' ')); + }, + error() {}, + setLevel() {}, + getLevel() { + return LogLevel.WARN; + }, + setName() {}, + }; + new WebhookTrigger('https://hooks.slack.com/services/T000/B000/abc', { logger }); + assert.strictEqual(warnings.length, 1); + assert.ok(warnings[0].includes('IncomingWebhook')); + }); + + it('should not warn when URL contains /triggers/', () => { + const warnings: string[] = []; + const logger = { + debug() {}, + info() {}, + warn(...msg: string[]) { + warnings.push(msg.join(' ')); + }, + error() {}, + setLevel() {}, + getLevel() { + return LogLevel.WARN; + }, + setName() {}, + }; + new WebhookTrigger('https://hooks.slack.com/triggers/T000/abc', { logger }); + assert.strictEqual(warnings.length, 0); + }); }); describe('send()', () => {