Skip to content

Commit e6e8a01

Browse files
author
bgagent
committed
fix(fanout): match SlackApiError by name as well as instanceof (aws-samples#79 review aws-samples#7)
When a bundler ever duplicates the slack-notify module (rare with NodejsFunction tree-shaking but possible if dual-bundled), two distinct SlackApiError classes coexist and ``instanceof`` against one fails for instances of the other. The dispatcher would see a foreign-class SlackApiError, fall through to the rethrow branch, and the router would treat it as an infra rejection — flipping a channel-terminal swallow into infinite Lambda retries. Add an ``err.name === 'SlackApiError'`` fallback so the swallow branch fires either way. Mirrors the duck-typed ``GitHubCommentError`` check used elsewhere in the same handler. Test: synthesise a plain Error with name === 'SlackApiError' (NOT an instance of the mock's SlackApiError class) and assert batchItemFailures stays empty — proving the swallow path catches both shapes.
1 parent 1d7c169 commit e6e8a01

2 files changed

Lines changed: 34 additions & 2 deletions

File tree

cdk/src/handlers/fanout-task-events.ts

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -347,14 +347,24 @@ async function dispatchToSlack(event: FanOutEvent): Promise<void> {
347347
try {
348348
await dispatchSlackEvent(effectiveEvent, ddb);
349349
} catch (err) {
350-
if (err instanceof SlackApiError) {
350+
// Match SlackApiError by class OR by ``name`` so a bundler that
351+
// duplicates the slack-notify module (rare with NodejsFunction
352+
// tree-shaking but possible if the module ever gets dual-bundled)
353+
// can't make ``instanceof`` silently fail and turn a
354+
// channel-terminal swallow into an infinite Lambda retry loop.
355+
// Mirrors how ``GitHubCommentError`` is duck-typed by name in
356+
// dispatchToGitHubComment (PR #79 review #7).
357+
const isSlackApiErr =
358+
err instanceof SlackApiError
359+
|| (err instanceof Error && err.name === 'SlackApiError');
360+
if (isSlackApiErr) {
351361
logger.warn('[fanout/slack] Slack API error — swallowing per channel policy', {
352362
event: 'fanout.slack.api_error',
353363
task_id: event.task_id,
354364
event_id: event.event_id,
355365
event_type: event.event_type,
356366
effective_event_type: effectiveType,
357-
error: err.message,
367+
error: (err as Error).message,
358368
});
359369
return;
360370
}

cdk/test/handlers/fanout-task-events.test.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1185,6 +1185,28 @@ describe('fanout-task-events: Slack dispatcher (issue #64 migration)', () => {
11851185
};
11861186
await expect(handler(event)).resolves.toEqual({ batchItemFailures: [] });
11871187
});
1188+
1189+
test('SlackApiError matched by name even when instanceof fails (PR #79 review #7)', async () => {
1190+
// Defense-in-depth: if a bundler ever duplicates the slack-notify
1191+
// module, two distinct SlackApiError classes coexist and
1192+
// ``instanceof`` against one fails for instances of the other.
1193+
// The dispatcher must fall back to ``err.name === 'SlackApiError'``
1194+
// so a duplicated-class scenario doesn't flip the channel-terminal
1195+
// swallow into an infinite retry loop. Synthesise that exact
1196+
// shape: a plain Error with name === 'SlackApiError', NOT an
1197+
// instance of the mock's SlackApiError class.
1198+
const fakeForeignSlackApiError = new Error(
1199+
'slack chat.postMessage failed: not_authed',
1200+
);
1201+
fakeForeignSlackApiError.name = 'SlackApiError';
1202+
mockDispatchSlackEvent.mockReset().mockRejectedValueOnce(fakeForeignSlackApiError);
1203+
1204+
const event: DynamoDBStreamEvent = {
1205+
Records: [mkEvent('task_completed', 't-slack-foreign-class')],
1206+
};
1207+
// Must still be caught — record advances, no batchItemFailures.
1208+
await expect(handler(event)).resolves.toEqual({ batchItemFailures: [] });
1209+
});
11881210
});
11891211

11901212
// ---------------------------------------------------------------------------

0 commit comments

Comments
 (0)