mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-06 01:55:37 +00:00
fix(slack): remove socket reconnect attempt cap so gateway stays connected indefinitely (#73162)
Merged via squash.
Prepared head SHA: ac51979a7f
Co-authored-by: suboss87 <11032439+suboss87@users.noreply.github.com>
Co-authored-by: steipete <58493+steipete@users.noreply.github.com>
Reviewed-by: @steipete
This commit is contained in:
@@ -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.<accountId>.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
|
||||
|
||||
|
||||
@@ -16,6 +16,8 @@ type SlackProviderMonitor = (params: {
|
||||
|
||||
type SlackTestState = {
|
||||
config: Record<string, unknown>;
|
||||
appStartMock: Mock<(...args: unknown[]) => Promise<unknown>>;
|
||||
appStopMock: Mock<(...args: unknown[]) => Promise<unknown>>;
|
||||
sendMock: Mock<(...args: unknown[]) => Promise<unknown>>;
|
||||
replyMock: Mock<(...args: unknown[]) => unknown>;
|
||||
updateLastRouteMock: Mock<(...args: unknown[]) => unknown>;
|
||||
@@ -31,6 +33,8 @@ type SlackTestState = {
|
||||
|
||||
const slackTestState: SlackTestState = vi.hoisted(() => ({
|
||||
config: {} as Record<string, unknown>,
|
||||
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<string, unknown> = 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();
|
||||
|
||||
@@ -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);
|
||||
|
||||
68
extensions/slack/src/monitor/provider.reconnect-loop.test.ts
Normal file
68
extensions/slack/src/monitor/provider.reconnect-loop.test.ts
Normal file
@@ -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/∞"));
|
||||
},
|
||||
);
|
||||
});
|
||||
@@ -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"',
|
||||
);
|
||||
});
|
||||
|
||||
|
||||
@@ -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(),
|
||||
|
||||
@@ -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, ""));
|
||||
|
||||
Reference in New Issue
Block a user