Skip to content

Commit 01dcaba

Browse files
suboss87steipete
andauthored
fix(slack): remove socket reconnect attempt cap so gateway stays connected indefinitely (#73162)
Merged via squash. Prepared head SHA: ac51979 Co-authored-by: suboss87 <[email protected]> Co-authored-by: steipete <[email protected]> Reviewed-by: @steipete
1 parent 5b5e6ff commit 01dcaba

7 files changed

Lines changed: 105 additions & 43 deletions

File tree

docs/channels/slack.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -529,7 +529,7 @@ Notes:
529529
- `socketMode` is ignored in HTTP Request URL mode.
530530
- Base `channels.slack.socketMode` settings apply to all Slack accounts unless overridden. Per-account overrides use `channels.slack.accounts.<accountId>.socketMode`; because this is an object override, include every socket tuning field you want for that account.
531531
- Only `clientPingTimeout` has an OpenClaw default (`15000`). `serverPingTimeout` and `pingPongLoggingEnabled` are passed to the Slack SDK only when configured.
532-
- Socket Mode restart backoff starts around 2 seconds and caps around 30 seconds. Consecutive recoverable start/start-wait failures stop after 12 attempts; after a successful connection, later recoverable disconnects start a fresh retry cycle. Non-recoverable Slack auth errors such as `invalid_auth`, revoked tokens, or missing scopes fail fast instead of retrying forever.
532+
- Socket Mode restart backoff starts around 2 seconds and caps around 30 seconds. Recoverable start, start-wait, and disconnect failures retry until the channel stops. Permanent account and credential errors such as invalid auth, revoked tokens, or missing scopes fail fast instead of retrying forever.
533533

534534
## Manifest and scope checklist
535535

extensions/slack/src/monitor.test-helpers.ts

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@ type SlackProviderMonitor = (params: {
1616

1717
type SlackTestState = {
1818
config: Record<string, unknown>;
19+
appStartMock: Mock<(...args: unknown[]) => Promise<unknown>>;
20+
appStopMock: Mock<(...args: unknown[]) => Promise<unknown>>;
1921
sendMock: Mock<(...args: unknown[]) => Promise<unknown>>;
2022
replyMock: Mock<(...args: unknown[]) => unknown>;
2123
updateLastRouteMock: Mock<(...args: unknown[]) => unknown>;
@@ -31,6 +33,8 @@ type SlackTestState = {
3133

3234
const slackTestState: SlackTestState = vi.hoisted(() => ({
3335
config: {} as Record<string, unknown>,
36+
appStartMock: vi.fn(),
37+
appStopMock: vi.fn(),
3438
sendMock: vi.fn(),
3539
replyMock: vi.fn(),
3640
updateLastRouteMock: vi.fn(),
@@ -202,6 +206,8 @@ export const defaultSlackTestConfig = () => ({
202206
export function resetSlackTestState(config: Record<string, unknown> = defaultSlackTestConfig()) {
203207
clearSlackInboundDeliveryStateForTest();
204208
slackTestState.config = config;
209+
slackTestState.appStartMock.mockReset().mockResolvedValue(undefined);
210+
slackTestState.appStopMock.mockReset().mockResolvedValue(undefined);
205211
slackTestState.sendMock.mockReset().mockResolvedValue(undefined);
206212
slackTestState.replyMock.mockReset();
207213
slackTestState.updateLastRouteMock.mockReset();
@@ -338,8 +344,8 @@ vi.mock("@slack/bolt", () => {
338344
command() {
339345
/* no-op */
340346
}
341-
start = vi.fn().mockResolvedValue(undefined);
342-
stop = vi.fn().mockResolvedValue(undefined);
347+
start = (...args: unknown[]) => slackTestState.appStartMock(...args);
348+
stop = (...args: unknown[]) => slackTestState.appStopMock(...args);
343349
}
344350
class HTTPReceiver {
345351
requestListener = vi.fn();

extensions/slack/src/monitor/provider.auth-errors.test.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ describe("isNonRecoverableSlackAuthError", () => {
1111
"An API error occurred: not_authed",
1212
"An API error occurred: org_login_required",
1313
"An API error occurred: team_access_not_granted",
14+
"An API error occurred: user_removed_from_team",
15+
"An API error occurred: team_disabled",
1416
"An API error occurred: missing_scope",
1517
"An API error occurred: cannot_find_service",
1618
"An API error occurred: invalid_token",
@@ -38,6 +40,20 @@ describe("isNonRecoverableSlackAuthError", () => {
3840
expect(isNonRecoverableSlackAuthError(new Error(msg))).toBe(false);
3941
});
4042

43+
it.each([
44+
{
45+
code: "slack_webapi_request_error",
46+
original: new Error("ECONNRESET"),
47+
},
48+
{
49+
code: "slack_webapi_http_error",
50+
statusCode: 503,
51+
statusMessage: "Service Unavailable",
52+
},
53+
])("returns false for recoverable Slack Web API errors", (error) => {
54+
expect(isNonRecoverableSlackAuthError(error)).toBe(false);
55+
});
56+
4157
it("returns false for non-error values", () => {
4258
expect(isNonRecoverableSlackAuthError(null)).toBe(false);
4359
expect(isNonRecoverableSlackAuthError(undefined)).toBe(false);
Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
// Slack tests cover provider reconnect loop behavior.
2+
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
3+
import { getSlackTestState, resetSlackTestState } from "../monitor.test-helpers.js";
4+
5+
const { monitorSlackProvider } = await import("./provider.js");
6+
const slackTestState = getSlackTestState();
7+
8+
describe("slack socket reconnect loop", () => {
9+
beforeEach(() => {
10+
resetSlackTestState();
11+
vi.useFakeTimers();
12+
});
13+
14+
afterEach(() => {
15+
vi.useRealTimers();
16+
});
17+
18+
it.each([
19+
["network error", () => new Error("ECONNRESET")],
20+
[
21+
"Slack Web API request error",
22+
() => ({
23+
code: "slack_webapi_request_error",
24+
original: new Error("ECONNRESET"),
25+
}),
26+
],
27+
[
28+
"Slack Web API HTTP error",
29+
() => ({
30+
code: "slack_webapi_http_error",
31+
statusCode: 503,
32+
statusMessage: "Service Unavailable",
33+
}),
34+
],
35+
])(
36+
"continues after thirteen consecutive recoverable %s failures",
37+
async (_label, createError) => {
38+
const controller = new AbortController();
39+
const runtimeError = vi.fn();
40+
let attempts = 0;
41+
slackTestState.appStartMock.mockImplementation(async () => {
42+
attempts += 1;
43+
if (attempts <= 13) {
44+
throw createError();
45+
}
46+
controller.abort();
47+
});
48+
49+
const run = monitorSlackProvider({
50+
botToken: "bot-token",
51+
appToken: "app-token",
52+
abortSignal: controller.signal,
53+
config: slackTestState.config,
54+
runtime: {
55+
log: vi.fn(),
56+
error: runtimeError,
57+
exit: vi.fn(),
58+
},
59+
});
60+
61+
await vi.runAllTimersAsync();
62+
await expect(run).resolves.toBeUndefined();
63+
64+
expect(slackTestState.appStartMock).toHaveBeenCalledTimes(14);
65+
expect(runtimeError).toHaveBeenCalledWith(expect.stringContaining("retry 13/∞"));
66+
},
67+
);
68+
});

extensions/slack/src/monitor/provider.reconnect.test.ts

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -104,15 +104,14 @@ describe("slack socket reconnect helpers", () => {
104104
});
105105
});
106106

107-
it("formats recoverable disconnects as a single reconnect status line", () => {
107+
it("formats recoverable disconnects beyond the former cap as unlimited", () => {
108108
expect(
109109
formatSlackSocketReconnectMessage({
110110
event: "disconnect",
111-
attempt: 1,
112-
maxAttempts: 12,
111+
attempt: 13,
113112
delayMs: 2_340,
114113
}),
115-
).toBe("slack socket disconnected (disconnect); reconnecting in 2s (attempt 1/12)");
114+
).toBe("slack socket disconnected (disconnect); reconnecting in 2s (attempt 13/∞)");
116115
});
117116

118117
it("formats missing and unserializable socket errors without leaking undefined", () => {
@@ -146,27 +145,25 @@ describe("slack socket reconnect helpers", () => {
146145
it("formats socket start retries with an explicit reason field", () => {
147146
expect(
148147
formatSlackSocketStartRetryMessage({
149-
attempt: 1,
150-
maxAttempts: 12,
148+
attempt: 13,
151149
delayMs: 2_340,
152150
error: undefined,
153151
}),
154152
).toBe(
155-
'slack socket mode failed to start; retry 1/12 in 2s reason="Slack Socket Mode start failed without error detail"',
153+
'slack socket mode failed to start; retry 13/∞ in 2s reason="Slack Socket Mode start failed without error detail"',
156154
);
157155
});
158156

159157
it("includes last SDK log context when start errors have no detail", () => {
160158
expect(
161159
formatSlackSocketStartRetryMessage({
162160
attempt: 1,
163-
maxAttempts: 12,
164161
delayMs: 2_340,
165162
error: undefined,
166163
sdkContext: "socket-mode:SlackWebSocket:1 Failed to retrieve WSS URL",
167164
}),
168165
).toBe(
169-
'slack socket mode failed to start; retry 1/12 in 2s reason="Slack Socket Mode start failed without error detail; last SDK log: socket-mode:SlackWebSocket:1 Failed to retrieve WSS URL"',
166+
'slack socket mode failed to start; retry 1/ in 2s reason="Slack Socket Mode start failed without error detail; last SDK log: socket-mode:SlackWebSocket:1 Failed to retrieve WSS URL"',
170167
);
171168
});
172169

extensions/slack/src/monitor/provider.ts

Lines changed: 3 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -116,29 +116,25 @@ function resolveStableSlackUserAllowlistEntries(entries: string[]): SlackUserRes
116116
export function formatSlackSocketReconnectMessage(params: {
117117
event: string;
118118
attempt: number;
119-
maxAttempts: number;
120119
delayMs: number;
121120
error?: unknown;
122121
}) {
123-
const maxAttempts = params.maxAttempts > 0 ? String(params.maxAttempts) : "∞";
124122
const suffix = params.error ? ` (${formatUnknownError(params.error)})` : "";
125-
return `slack socket disconnected (${params.event}); reconnecting in ${Math.round(params.delayMs / 1000)}s (attempt ${params.attempt}/${maxAttempts})${suffix}`;
123+
return `slack socket disconnected (${params.event}); reconnecting in ${Math.round(params.delayMs / 1000)}s (attempt ${params.attempt}/)${suffix}`;
126124
}
127125

128126
export function formatSlackSocketStartRetryMessage(params: {
129127
attempt: number;
130-
maxAttempts: number;
131128
delayMs: number;
132129
error: unknown;
133130
sdkContext?: string;
134131
}) {
135-
const maxAttempts = params.maxAttempts > 0 ? String(params.maxAttempts) : "∞";
136132
const reason = formatUnknownError(
137133
params.error,
138134
"Slack Socket Mode start failed without error detail",
139135
);
140136
const sdkContext = params.sdkContext?.trim() ? `; last SDK log: ${params.sdkContext.trim()}` : "";
141-
return `slack socket mode failed to start; retry ${params.attempt}/${maxAttempts} in ${Math.round(params.delayMs / 1000)}s reason="${reason}${sdkContext}"`;
137+
return `slack socket mode failed to start; retry ${params.attempt}/ in ${Math.round(params.delayMs / 1000)}s reason="${reason}${sdkContext}"`;
142138
}
143139

144140
function parseApiAppIdFromAppToken(raw?: string) {
@@ -568,7 +564,7 @@ export async function monitorSlackProvider(opts: MonitorSlackOpts = {}) {
568564
}
569565
publishSlackDisconnectedStatus(opts.setStatus, disconnect.error);
570566

571-
// Bail immediately on non-recoverable auth errors during reconnect too.
567+
// Permanent account and credential failures need operator action.
572568
if (disconnect.error && isNonRecoverableSlackAuthError(disconnect.error)) {
573569
runtime.error?.(
574570
`slack socket mode disconnected due to non-recoverable auth error — skipping channel (${formatUnknownError(disconnect.error)})`,
@@ -579,22 +575,12 @@ export async function monitorSlackProvider(opts: MonitorSlackOpts = {}) {
579575
}
580576

581577
reconnectAttempts += 1;
582-
if (
583-
SLACK_SOCKET_RECONNECT_POLICY.maxAttempts > 0 &&
584-
reconnectAttempts >= SLACK_SOCKET_RECONNECT_POLICY.maxAttempts
585-
) {
586-
throw new Error(
587-
`Slack socket mode reconnect max attempts reached (${reconnectAttempts}/${SLACK_SOCKET_RECONNECT_POLICY.maxAttempts}) after ${disconnect.event}`,
588-
);
589-
}
590-
591578
const delayMs = computeBackoff(SLACK_SOCKET_RECONNECT_POLICY, reconnectAttempts);
592579
runtime.log?.(
593580
warn(
594581
formatSlackSocketReconnectMessage({
595582
event: disconnect.event,
596583
attempt: reconnectAttempts,
597-
maxAttempts: SLACK_SOCKET_RECONNECT_POLICY.maxAttempts,
598584
delayMs,
599585
error: disconnect.error,
600586
}),
@@ -607,26 +593,17 @@ export async function monitorSlackProvider(opts: MonitorSlackOpts = {}) {
607593
break;
608594
}
609595
} catch (err) {
610-
// Auth errors (account_inactive, invalid_auth, etc.) are permanent —
611-
// retrying will never succeed and blocks the entire gateway. Fail fast.
612596
if (isNonRecoverableSlackAuthError(err)) {
613597
runtime.error?.(
614598
`slack socket mode failed to start due to non-recoverable auth error — skipping channel (${formatUnknownError(err)})`,
615599
);
616600
throw err;
617601
}
618602
reconnectAttempts += 1;
619-
if (
620-
SLACK_SOCKET_RECONNECT_POLICY.maxAttempts > 0 &&
621-
reconnectAttempts >= SLACK_SOCKET_RECONNECT_POLICY.maxAttempts
622-
) {
623-
throw err;
624-
}
625603
const delayMs = computeBackoff(SLACK_SOCKET_RECONNECT_POLICY, reconnectAttempts);
626604
runtime.error?.(
627605
formatSlackSocketStartRetryMessage({
628606
attempt: reconnectAttempts,
629-
maxAttempts: SLACK_SOCKET_RECONNECT_POLICY.maxAttempts,
630607
delayMs,
631608
error: err,
632609
sdkContext: socketModeLogger.getLastMessage(),

extensions/slack/src/monitor/reconnect-policy.ts

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,14 @@
22
import { formatSlackError } from "../errors.js";
33

44
const SLACK_AUTH_ERROR_RE =
5-
/account_inactive|invalid_auth|token_revoked|token_expired|not_authed|org_login_required|team_access_not_granted|missing_scope|cannot_find_service|invalid_token/i;
5+
/account_inactive|invalid_auth|token_revoked|token_expired|not_authed|org_login_required|team_access_not_granted|user_removed_from_team|team_disabled|missing_scope|cannot_find_service|invalid_token/i;
66
const NO_ERROR_DETAIL = "no error detail";
77

88
export const SLACK_SOCKET_RECONNECT_POLICY = {
99
initialMs: 2_000,
1010
maxMs: 30_000,
1111
factor: 1.8,
1212
jitter: 0.25,
13-
maxAttempts: 12,
1413
} as const;
1514

1615
type SlackSocketDisconnectEvent = "disconnect" | "unable_to_socket_mode_start" | "error";
@@ -88,9 +87,8 @@ export function waitForSlackSocketDisconnect(
8887
}
8988

9089
/**
91-
* Detect non-recoverable Slack API / auth errors that should NOT be retried.
92-
* These indicate permanent credential problems (revoked bot, deactivated account, etc.)
93-
* and retrying will never succeed — continuing to retry blocks the entire gateway.
90+
* Detect permanent Slack account and credential failures.
91+
* Transient request and HTTP failures stay in OpenClaw's reconnect loop.
9492
*/
9593
export function isNonRecoverableSlackAuthError(error: unknown): boolean {
9694
return SLACK_AUTH_ERROR_RE.test(formatUnknownError(error, ""));

0 commit comments

Comments
 (0)