diff --git a/docs/channels/slack.md b/docs/channels/slack.md index ed459d3dc821..2f62cf73ed68 100644 --- a/docs/channels/slack.md +++ b/docs/channels/slack.md @@ -529,7 +529,7 @@ Notes: - `socketMode` is ignored in HTTP Request URL mode. - Base `channels.slack.socketMode` settings apply to all Slack accounts unless overridden. Per-account overrides use `channels.slack.accounts..socketMode`; because this is an object override, include every socket tuning field you want for that account. - Only `clientPingTimeout` has an OpenClaw default (`15000`). `serverPingTimeout` and `pingPongLoggingEnabled` are passed to the Slack SDK only when configured. -- 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. +- 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. ## Manifest and scope checklist diff --git a/extensions/slack/src/monitor.test-helpers.ts b/extensions/slack/src/monitor.test-helpers.ts index 1ffd0bd70b61..16531b256644 100644 --- a/extensions/slack/src/monitor.test-helpers.ts +++ b/extensions/slack/src/monitor.test-helpers.ts @@ -16,6 +16,8 @@ type SlackProviderMonitor = (params: { type SlackTestState = { config: Record; + appStartMock: Mock<(...args: unknown[]) => Promise>; + appStopMock: Mock<(...args: unknown[]) => Promise>; sendMock: Mock<(...args: unknown[]) => Promise>; replyMock: Mock<(...args: unknown[]) => unknown>; updateLastRouteMock: Mock<(...args: unknown[]) => unknown>; @@ -31,6 +33,8 @@ type SlackTestState = { const slackTestState: SlackTestState = vi.hoisted(() => ({ config: {} as Record, + appStartMock: vi.fn(), + appStopMock: vi.fn(), sendMock: vi.fn(), replyMock: vi.fn(), updateLastRouteMock: vi.fn(), @@ -202,6 +206,8 @@ export const defaultSlackTestConfig = () => ({ export function resetSlackTestState(config: Record = defaultSlackTestConfig()) { clearSlackInboundDeliveryStateForTest(); slackTestState.config = config; + slackTestState.appStartMock.mockReset().mockResolvedValue(undefined); + slackTestState.appStopMock.mockReset().mockResolvedValue(undefined); slackTestState.sendMock.mockReset().mockResolvedValue(undefined); slackTestState.replyMock.mockReset(); slackTestState.updateLastRouteMock.mockReset(); @@ -338,8 +344,8 @@ vi.mock("@slack/bolt", () => { command() { /* no-op */ } - start = vi.fn().mockResolvedValue(undefined); - stop = vi.fn().mockResolvedValue(undefined); + start = (...args: unknown[]) => slackTestState.appStartMock(...args); + stop = (...args: unknown[]) => slackTestState.appStopMock(...args); } class HTTPReceiver { requestListener = vi.fn(); diff --git a/extensions/slack/src/monitor/provider.auth-errors.test.ts b/extensions/slack/src/monitor/provider.auth-errors.test.ts index 8a541c3b357e..737697784e35 100644 --- a/extensions/slack/src/monitor/provider.auth-errors.test.ts +++ b/extensions/slack/src/monitor/provider.auth-errors.test.ts @@ -11,6 +11,8 @@ describe("isNonRecoverableSlackAuthError", () => { "An API error occurred: not_authed", "An API error occurred: org_login_required", "An API error occurred: team_access_not_granted", + "An API error occurred: user_removed_from_team", + "An API error occurred: team_disabled", "An API error occurred: missing_scope", "An API error occurred: cannot_find_service", "An API error occurred: invalid_token", @@ -38,6 +40,20 @@ describe("isNonRecoverableSlackAuthError", () => { expect(isNonRecoverableSlackAuthError(new Error(msg))).toBe(false); }); + it.each([ + { + code: "slack_webapi_request_error", + original: new Error("ECONNRESET"), + }, + { + code: "slack_webapi_http_error", + statusCode: 503, + statusMessage: "Service Unavailable", + }, + ])("returns false for recoverable Slack Web API errors", (error) => { + expect(isNonRecoverableSlackAuthError(error)).toBe(false); + }); + it("returns false for non-error values", () => { expect(isNonRecoverableSlackAuthError(null)).toBe(false); expect(isNonRecoverableSlackAuthError(undefined)).toBe(false); diff --git a/extensions/slack/src/monitor/provider.reconnect-loop.test.ts b/extensions/slack/src/monitor/provider.reconnect-loop.test.ts new file mode 100644 index 000000000000..65f8a48c0c2f --- /dev/null +++ b/extensions/slack/src/monitor/provider.reconnect-loop.test.ts @@ -0,0 +1,68 @@ +// Slack tests cover provider reconnect loop behavior. +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { getSlackTestState, resetSlackTestState } from "../monitor.test-helpers.js"; + +const { monitorSlackProvider } = await import("./provider.js"); +const slackTestState = getSlackTestState(); + +describe("slack socket reconnect loop", () => { + beforeEach(() => { + resetSlackTestState(); + vi.useFakeTimers(); + }); + + afterEach(() => { + vi.useRealTimers(); + }); + + it.each([ + ["network error", () => new Error("ECONNRESET")], + [ + "Slack Web API request error", + () => ({ + code: "slack_webapi_request_error", + original: new Error("ECONNRESET"), + }), + ], + [ + "Slack Web API HTTP error", + () => ({ + code: "slack_webapi_http_error", + statusCode: 503, + statusMessage: "Service Unavailable", + }), + ], + ])( + "continues after thirteen consecutive recoverable %s failures", + async (_label, createError) => { + const controller = new AbortController(); + const runtimeError = vi.fn(); + let attempts = 0; + slackTestState.appStartMock.mockImplementation(async () => { + attempts += 1; + if (attempts <= 13) { + throw createError(); + } + controller.abort(); + }); + + const run = monitorSlackProvider({ + botToken: "bot-token", + appToken: "app-token", + abortSignal: controller.signal, + config: slackTestState.config, + runtime: { + log: vi.fn(), + error: runtimeError, + exit: vi.fn(), + }, + }); + + await vi.runAllTimersAsync(); + await expect(run).resolves.toBeUndefined(); + + expect(slackTestState.appStartMock).toHaveBeenCalledTimes(14); + expect(runtimeError).toHaveBeenCalledWith(expect.stringContaining("retry 13/∞")); + }, + ); +}); diff --git a/extensions/slack/src/monitor/provider.reconnect.test.ts b/extensions/slack/src/monitor/provider.reconnect.test.ts index 3f8c6f84dc48..12133b61ee2f 100644 --- a/extensions/slack/src/monitor/provider.reconnect.test.ts +++ b/extensions/slack/src/monitor/provider.reconnect.test.ts @@ -104,15 +104,14 @@ describe("slack socket reconnect helpers", () => { }); }); - it("formats recoverable disconnects as a single reconnect status line", () => { + it("formats recoverable disconnects beyond the former cap as unlimited", () => { expect( formatSlackSocketReconnectMessage({ event: "disconnect", - attempt: 1, - maxAttempts: 12, + attempt: 13, delayMs: 2_340, }), - ).toBe("slack socket disconnected (disconnect); reconnecting in 2s (attempt 1/12)"); + ).toBe("slack socket disconnected (disconnect); reconnecting in 2s (attempt 13/∞)"); }); it("formats missing and unserializable socket errors without leaking undefined", () => { @@ -146,13 +145,12 @@ describe("slack socket reconnect helpers", () => { it("formats socket start retries with an explicit reason field", () => { expect( formatSlackSocketStartRetryMessage({ - attempt: 1, - maxAttempts: 12, + attempt: 13, delayMs: 2_340, error: undefined, }), ).toBe( - 'slack socket mode failed to start; retry 1/12 in 2s reason="Slack Socket Mode start failed without error detail"', + 'slack socket mode failed to start; retry 13/∞ in 2s reason="Slack Socket Mode start failed without error detail"', ); }); @@ -160,13 +158,12 @@ describe("slack socket reconnect helpers", () => { expect( formatSlackSocketStartRetryMessage({ attempt: 1, - maxAttempts: 12, delayMs: 2_340, error: undefined, sdkContext: "socket-mode:SlackWebSocket:1 Failed to retrieve WSS URL", }), ).toBe( - '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"', + '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"', ); }); diff --git a/extensions/slack/src/monitor/provider.ts b/extensions/slack/src/monitor/provider.ts index be8de17e333d..f3feb4d8fa40 100644 --- a/extensions/slack/src/monitor/provider.ts +++ b/extensions/slack/src/monitor/provider.ts @@ -116,29 +116,25 @@ function resolveStableSlackUserAllowlistEntries(entries: string[]): SlackUserRes export function formatSlackSocketReconnectMessage(params: { event: string; attempt: number; - maxAttempts: number; delayMs: number; error?: unknown; }) { - const maxAttempts = params.maxAttempts > 0 ? String(params.maxAttempts) : "∞"; const suffix = params.error ? ` (${formatUnknownError(params.error)})` : ""; - return `slack socket disconnected (${params.event}); reconnecting in ${Math.round(params.delayMs / 1000)}s (attempt ${params.attempt}/${maxAttempts})${suffix}`; + return `slack socket disconnected (${params.event}); reconnecting in ${Math.round(params.delayMs / 1000)}s (attempt ${params.attempt}/∞)${suffix}`; } export function formatSlackSocketStartRetryMessage(params: { attempt: number; - maxAttempts: number; delayMs: number; error: unknown; sdkContext?: string; }) { - const maxAttempts = params.maxAttempts > 0 ? String(params.maxAttempts) : "∞"; const reason = formatUnknownError( params.error, "Slack Socket Mode start failed without error detail", ); const sdkContext = params.sdkContext?.trim() ? `; last SDK log: ${params.sdkContext.trim()}` : ""; - return `slack socket mode failed to start; retry ${params.attempt}/${maxAttempts} in ${Math.round(params.delayMs / 1000)}s reason="${reason}${sdkContext}"`; + return `slack socket mode failed to start; retry ${params.attempt}/∞ in ${Math.round(params.delayMs / 1000)}s reason="${reason}${sdkContext}"`; } function parseApiAppIdFromAppToken(raw?: string) { @@ -568,7 +564,7 @@ export async function monitorSlackProvider(opts: MonitorSlackOpts = {}) { } publishSlackDisconnectedStatus(opts.setStatus, disconnect.error); - // Bail immediately on non-recoverable auth errors during reconnect too. + // Permanent account and credential failures need operator action. if (disconnect.error && isNonRecoverableSlackAuthError(disconnect.error)) { runtime.error?.( `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 = {}) { } reconnectAttempts += 1; - if ( - SLACK_SOCKET_RECONNECT_POLICY.maxAttempts > 0 && - reconnectAttempts >= SLACK_SOCKET_RECONNECT_POLICY.maxAttempts - ) { - throw new Error( - `Slack socket mode reconnect max attempts reached (${reconnectAttempts}/${SLACK_SOCKET_RECONNECT_POLICY.maxAttempts}) after ${disconnect.event}`, - ); - } - const delayMs = computeBackoff(SLACK_SOCKET_RECONNECT_POLICY, reconnectAttempts); runtime.log?.( warn( formatSlackSocketReconnectMessage({ event: disconnect.event, attempt: reconnectAttempts, - maxAttempts: SLACK_SOCKET_RECONNECT_POLICY.maxAttempts, delayMs, error: disconnect.error, }), @@ -607,8 +593,6 @@ export async function monitorSlackProvider(opts: MonitorSlackOpts = {}) { break; } } catch (err) { - // Auth errors (account_inactive, invalid_auth, etc.) are permanent — - // retrying will never succeed and blocks the entire gateway. Fail fast. if (isNonRecoverableSlackAuthError(err)) { runtime.error?.( `slack socket mode failed to start due to non-recoverable auth error — skipping channel (${formatUnknownError(err)})`, @@ -616,17 +600,10 @@ export async function monitorSlackProvider(opts: MonitorSlackOpts = {}) { throw err; } reconnectAttempts += 1; - if ( - SLACK_SOCKET_RECONNECT_POLICY.maxAttempts > 0 && - reconnectAttempts >= SLACK_SOCKET_RECONNECT_POLICY.maxAttempts - ) { - throw err; - } const delayMs = computeBackoff(SLACK_SOCKET_RECONNECT_POLICY, reconnectAttempts); runtime.error?.( formatSlackSocketStartRetryMessage({ attempt: reconnectAttempts, - maxAttempts: SLACK_SOCKET_RECONNECT_POLICY.maxAttempts, delayMs, error: err, sdkContext: socketModeLogger.getLastMessage(), diff --git a/extensions/slack/src/monitor/reconnect-policy.ts b/extensions/slack/src/monitor/reconnect-policy.ts index 521cf8a189bd..6206c9a5bfa5 100644 --- a/extensions/slack/src/monitor/reconnect-policy.ts +++ b/extensions/slack/src/monitor/reconnect-policy.ts @@ -2,7 +2,7 @@ import { formatSlackError } from "../errors.js"; const SLACK_AUTH_ERROR_RE = - /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; + /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; const NO_ERROR_DETAIL = "no error detail"; export const SLACK_SOCKET_RECONNECT_POLICY = { @@ -10,7 +10,6 @@ export const SLACK_SOCKET_RECONNECT_POLICY = { maxMs: 30_000, factor: 1.8, jitter: 0.25, - maxAttempts: 12, } as const; type SlackSocketDisconnectEvent = "disconnect" | "unable_to_socket_mode_start" | "error"; @@ -88,9 +87,8 @@ export function waitForSlackSocketDisconnect( } /** - * Detect non-recoverable Slack API / auth errors that should NOT be retried. - * These indicate permanent credential problems (revoked bot, deactivated account, etc.) - * and retrying will never succeed — continuing to retry blocks the entire gateway. + * Detect permanent Slack account and credential failures. + * Transient request and HTTP failures stay in OpenClaw's reconnect loop. */ export function isNonRecoverableSlackAuthError(error: unknown): boolean { return SLACK_AUTH_ERROR_RE.test(formatUnknownError(error, ""));