Skip to content

Commit a29c157

Browse files
author
bgagent
committed
fix(fanout): extend TERMINAL_SLACK_API_ERRORS with permission codes (aws-samples#79 review aws-samples#8)
Original set omitted documented Slack permission/scope failures. Codes outside the set fall to the retryable branch, so a misconfiguration like ``ekm_access_denied`` or ``missing_scope`` would burn 3 Lambda retries before DLQ on every event — even though the failure is fundamentally a configuration bug that no retry can clear. Adds: - Permission/scope: missing_scope, ekm_access_denied, team_access_not_granted, posting_to_general_channel_denied - Payload shape: invalid_arguments Reorganized the set into commented blocks (channel-shape, auth, permission/scope, payload-shape) so future additions go in the right bucket and the rationale stays visible. Test coverage: parametrized over the full TERMINAL_SLACK_API_ERRORS set (21 codes) — every one must throw SlackApiError so the router swallows it. The existing retryable test.each remains intact and covers the negative-class case (codes outside the set throw a plain Error and escalate to retry).
1 parent e6e8a01 commit a29c157

2 files changed

Lines changed: 47 additions & 8 deletions

File tree

cdk/src/handlers/slack-notify.ts

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -125,22 +125,34 @@ export class SlackApiError extends Error {
125125
* ``service_unavailable`` doesn't get permanently dropped.
126126
*/
127127
const TERMINAL_SLACK_API_ERRORS: ReadonlySet<string> = new Set([
128+
// Channel-shape failures.
128129
'channel_not_found',
129130
'not_in_channel',
130131
'is_archived',
132+
'message_not_found',
133+
// Auth failures.
131134
'not_authed',
132135
'invalid_auth',
133136
'token_revoked',
134137
'token_expired',
135138
'account_inactive',
139+
// Permission / scope failures (PR #79 review #8): each of these
140+
// means a configuration fix is required before any retry can
141+
// succeed, so swallow them as terminal and let operators alert on
142+
// the dedicated ``fanout.slack.api_error`` warn rate.
136143
'no_permission',
144+
'missing_scope',
145+
'restricted_action',
146+
'ekm_access_denied',
147+
'team_access_not_granted',
148+
'posting_to_general_channel_denied',
149+
'as_user_not_supported',
150+
// Payload-shape failures.
137151
'invalid_blocks',
138152
'invalid_blocks_format',
153+
'invalid_arguments',
139154
'msg_too_long',
140155
'too_many_attachments',
141-
'restricted_action',
142-
'as_user_not_supported',
143-
'message_not_found',
144156
]);
145157

146158
/** Tag a Slack ``!result.ok`` error as terminal vs retryable so the

cdk/test/handlers/slack-notify.test.ts

Lines changed: 32 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -123,10 +123,37 @@ describe('dispatchSlackEvent', () => {
123123
expect(fetchMock).not.toHaveBeenCalled();
124124
});
125125

126-
test('throws a tagged SlackApiError on a TERMINAL Slack code (router swallows)', async () => {
127-
// Slack errors like channel_not_found are terminal — retrying
128-
// won't change the outcome. Throw SlackApiError so the router
129-
// catches and logs at warn instead of escalating to batch retry.
126+
test.each([
127+
// Channel + message shape.
128+
'channel_not_found',
129+
'not_in_channel',
130+
'is_archived',
131+
'message_not_found',
132+
// Auth.
133+
'not_authed',
134+
'invalid_auth',
135+
'token_revoked',
136+
'token_expired',
137+
'account_inactive',
138+
// Permission / scope (PR #79 review #8).
139+
'no_permission',
140+
'missing_scope',
141+
'restricted_action',
142+
'ekm_access_denied',
143+
'team_access_not_granted',
144+
'posting_to_general_channel_denied',
145+
'as_user_not_supported',
146+
// Payload shape.
147+
'invalid_blocks',
148+
'invalid_blocks_format',
149+
'invalid_arguments',
150+
'msg_too_long',
151+
'too_many_attachments',
152+
])('throws a tagged SlackApiError on terminal Slack code %s (router swallows)', async (slackErrorCode) => {
153+
// Pre-PR-#79-review the set was narrower; permission/scope
154+
// codes (ekm_access_denied, missing_scope, etc.) used to be
155+
// classified retryable and would burn 3 retries before DLQ on
156+
// a misconfiguration that no retry can fix.
130157
ddbSend.mockResolvedValueOnce({
131158
Item: {
132159
task_id: 't1',
@@ -136,7 +163,7 @@ describe('dispatchSlackEvent', () => {
136163
});
137164
fetchMock.mockResolvedValueOnce({
138165
ok: true,
139-
json: () => Promise.resolve({ ok: false, error: 'channel_not_found' }),
166+
json: () => Promise.resolve({ ok: false, error: slackErrorCode }),
140167
});
141168

142169
await expect(

0 commit comments

Comments
 (0)