mirror of
https://github.com/openclaw/openclaw.git
synced 2026-07-21 10:16:44 +00:00
fix(slack): add timeout to startup auth.test requests (#105893)
* fix(slack): add timeout to startup auth.test requests * test(slack): format auth startup timeout proof * fix(slack): keep startup timeout test helper private * fix(slack): keep auth timeout state lean * test(slack): settle stalled auth test during cleanup * fix(slack): keep startup timeout within LOC guard * fix(slack): terminate stalled startup auth checks * test(slack): align startup logger mock type
This commit is contained in:
@@ -18,6 +18,7 @@ vi.mock("@slack/web-api", () => {
|
||||
});
|
||||
|
||||
let createSlackWebClient: typeof import("./client.js").createSlackWebClient;
|
||||
let createSlackStartupAuthClient: typeof import("./client.js").createSlackStartupAuthClient;
|
||||
let createSlackLookupClient: typeof import("./client.js").createSlackLookupClient;
|
||||
let createSlackWriteClient: typeof import("./client.js").createSlackWriteClient;
|
||||
let createSlackTokenCacheKey: typeof import("./client.js").createSlackTokenCacheKey;
|
||||
@@ -31,8 +32,10 @@ let WebClient: ReturnType<typeof vi.fn>;
|
||||
|
||||
const SLACK_API_URL_KEYS = ["SLACK_API_URL", "OPENCLAW_SLACK_API_URL"] as const;
|
||||
const PROXY_KEYS = [
|
||||
"ALL_PROXY",
|
||||
"HTTPS_PROXY",
|
||||
"HTTP_PROXY",
|
||||
"all_proxy",
|
||||
"https_proxy",
|
||||
"http_proxy",
|
||||
"NO_PROXY",
|
||||
@@ -94,6 +97,7 @@ beforeAll(async () => {
|
||||
const slackWebApi = await import("@slack/web-api");
|
||||
({
|
||||
createSlackWebClient,
|
||||
createSlackStartupAuthClient,
|
||||
createSlackLookupClient,
|
||||
createSlackWriteClient,
|
||||
createSlackTokenCacheKey,
|
||||
@@ -186,6 +190,23 @@ describe("slack web client config", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("bounds startup auth while preserving listener transport options", () => {
|
||||
const customAgent = {} as never;
|
||||
|
||||
createSlackStartupAuthClient("xoxb-startup", {
|
||||
agent: customAgent,
|
||||
slackApiUrl: "https://slack.test/api/",
|
||||
});
|
||||
|
||||
expect(WebClient).toHaveBeenCalledWith("xoxb-startup", {
|
||||
agent: customAgent,
|
||||
rejectRateLimitedCalls: true,
|
||||
retryConfig: { retries: 0 },
|
||||
slackApiUrl: "https://slack.test/api/",
|
||||
timeout: 10_000,
|
||||
});
|
||||
});
|
||||
|
||||
it("applies the default retry config when constructing a client without proxy env", () => {
|
||||
clearProxyEnvForTest();
|
||||
try {
|
||||
|
||||
@@ -29,6 +29,17 @@ export function createSlackWebClient(token: string, options: WebClientOptions =
|
||||
return new WebClient(token, resolveSlackWebClientOptions(options));
|
||||
}
|
||||
|
||||
export function createSlackStartupAuthClient(token: string, options: WebClientOptions = {}) {
|
||||
// Startup degrades after auth.test fails, so terminate this one-shot request without
|
||||
// imposing the same short deadline on Bolt's long-lived client.
|
||||
return createSlackWebClient(token, {
|
||||
...options,
|
||||
rejectRateLimitedCalls: true,
|
||||
retryConfig: { retries: 0 },
|
||||
timeout: 10_000,
|
||||
});
|
||||
}
|
||||
|
||||
export function createSlackLookupClient(token: string, options: SlackLookupClientOptions = {}) {
|
||||
return new WebClient(token, resolveSlackLookupClientOptions(options));
|
||||
}
|
||||
|
||||
@@ -16,6 +16,7 @@ type SlackProviderMonitor = (params: {
|
||||
runtime?: RuntimeEnv;
|
||||
setStatus?: (next: Record<string, unknown>) => void;
|
||||
}) => Promise<unknown>;
|
||||
type SlackStartupAuthClientFactory = typeof import("./client.js").createSlackStartupAuthClient;
|
||||
|
||||
type SlackTestState = {
|
||||
config: Record<string, unknown>;
|
||||
@@ -33,6 +34,8 @@ type SlackTestState = {
|
||||
(params: { entries: string[] }) => Promise<Array<{ input: string; resolved: boolean }>>
|
||||
>;
|
||||
socketModeLogger?: { error: (...args: unknown[]) => void };
|
||||
createSlackStartupAuthClientMock: Mock<SlackStartupAuthClientFactory>;
|
||||
createSlackStartupAuthClientActual?: SlackStartupAuthClientFactory;
|
||||
};
|
||||
|
||||
const slackTestState: SlackTestState = vi.hoisted(() => ({
|
||||
@@ -49,6 +52,7 @@ const slackTestState: SlackTestState = vi.hoisted(() => ({
|
||||
upsertPairingRequestMock: vi.fn(),
|
||||
resolveSlackUserAllowlistMock: vi.fn(),
|
||||
socketModeLogger: undefined,
|
||||
createSlackStartupAuthClientMock: vi.fn(),
|
||||
}));
|
||||
|
||||
const slackInboundDeliveryTestCache = resolveGlobalDedupeCache(
|
||||
@@ -61,6 +65,14 @@ const slackInboundDeliveryTestCache = resolveGlobalDedupeCache(
|
||||
|
||||
export const getSlackTestState = (): SlackTestState => slackTestState;
|
||||
|
||||
export function useRealSlackStartupAuthClientOnce(): void {
|
||||
const actual = slackTestState.createSlackStartupAuthClientActual;
|
||||
if (!actual) {
|
||||
throw new Error("real Slack WebClient factory is unavailable");
|
||||
}
|
||||
slackTestState.createSlackStartupAuthClientMock.mockImplementationOnce(actual);
|
||||
}
|
||||
|
||||
type SlackClient = {
|
||||
auth: { test: Mock<(...args: unknown[]) => Promise<Record<string, unknown>>> };
|
||||
conversations: {
|
||||
@@ -246,6 +258,9 @@ export function resetSlackTestState(config: Record<string, unknown> = defaultSla
|
||||
.mockImplementation(async ({ entries }) =>
|
||||
entries.map((input) => ({ input, resolved: false })),
|
||||
);
|
||||
slackTestState.createSlackStartupAuthClientMock
|
||||
.mockReset()
|
||||
.mockReturnValue(getSlackClient() as unknown as ReturnType<SlackStartupAuthClientFactory>);
|
||||
const client = getSlackClient();
|
||||
client.auth.test.mockReset().mockResolvedValue({
|
||||
user_id: "bot-user",
|
||||
@@ -310,6 +325,16 @@ vi.mock("./resolve-users.js", () => ({
|
||||
slackTestState.resolveSlackUserAllowlistMock(params),
|
||||
}));
|
||||
|
||||
vi.mock("./client.js", async () => {
|
||||
const actual = await vi.importActual<typeof import("./client.js")>("./client.js");
|
||||
slackTestState.createSlackStartupAuthClientActual = actual.createSlackStartupAuthClient;
|
||||
return {
|
||||
...actual,
|
||||
createSlackStartupAuthClient: (...args: Parameters<SlackStartupAuthClientFactory>) =>
|
||||
slackTestState.createSlackStartupAuthClientMock(...args),
|
||||
};
|
||||
});
|
||||
|
||||
vi.mock("./monitor/send.runtime.js", () => {
|
||||
return {
|
||||
sendMessageSlack: (...args: unknown[]) => slackTestState.sendMock(...args),
|
||||
|
||||
@@ -1,5 +1,7 @@
|
||||
// Slack tests cover auth.test token handling during provider boot.
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import { createServer } from "node:http";
|
||||
import type { AddressInfo } from "node:net";
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import {
|
||||
getSlackClient,
|
||||
getSlackTestState,
|
||||
@@ -7,14 +9,63 @@ import {
|
||||
runSlackMessageOnce,
|
||||
startSlackMonitor,
|
||||
stopSlackMonitor,
|
||||
useRealSlackStartupAuthClientOnce,
|
||||
} from "../monitor.test-helpers.js";
|
||||
|
||||
const { monitorSlackProvider } = await import("./provider.js");
|
||||
|
||||
const PROXY_ENV_KEYS = [
|
||||
"ALL_PROXY",
|
||||
"HTTPS_PROXY",
|
||||
"HTTP_PROXY",
|
||||
"all_proxy",
|
||||
"https_proxy",
|
||||
"http_proxy",
|
||||
"NO_PROXY",
|
||||
"no_proxy",
|
||||
] as const;
|
||||
|
||||
async function startStalledSlackApiServer(events: string[]) {
|
||||
let requestCount = 0;
|
||||
let requestUrl: string | undefined;
|
||||
const server = createServer((request) => {
|
||||
requestCount += 1;
|
||||
requestUrl = request.url;
|
||||
events.push("request");
|
||||
request.resume();
|
||||
request.socket.once("close", () => {
|
||||
events.push("socket-closed");
|
||||
});
|
||||
});
|
||||
await new Promise<void>((resolve) => {
|
||||
server.listen(0, "127.0.0.1", resolve);
|
||||
});
|
||||
const address = server.address() as AddressInfo;
|
||||
return {
|
||||
apiUrl: `http://127.0.0.1:${address.port}/api/`,
|
||||
get requestCount() {
|
||||
return requestCount;
|
||||
},
|
||||
get requestUrl() {
|
||||
return requestUrl;
|
||||
},
|
||||
close: async () => {
|
||||
server.closeAllConnections();
|
||||
await new Promise<void>((resolve, reject) => {
|
||||
server.close((error) => (error ? reject(error) : resolve()));
|
||||
});
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
resetSlackTestState();
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
vi.unstubAllEnvs();
|
||||
});
|
||||
|
||||
describe("auth.test boot call", () => {
|
||||
it("does not pass the bot token in the call arguments", async () => {
|
||||
const monitor = startSlackMonitor(monitorSlackProvider);
|
||||
@@ -126,6 +177,77 @@ describe("auth.test boot call", () => {
|
||||
);
|
||||
});
|
||||
|
||||
it("continues startup after the startup auth client times out", async () => {
|
||||
const runtimeLog = vi.fn();
|
||||
const { appStartMock, createSlackStartupAuthClientMock } = getSlackTestState();
|
||||
vi.stubEnv("SLACK_API_URL", "https://slack.test/api/");
|
||||
vi.stubEnv("https_proxy", "http://proxy.test:3128");
|
||||
vi.stubEnv("no_proxy", "");
|
||||
getSlackClient().auth.test.mockRejectedValueOnce(
|
||||
new Error("A request error occurred: timeout of 10000ms exceeded"),
|
||||
);
|
||||
|
||||
const monitor = startSlackMonitor(monitorSlackProvider, {
|
||||
runtime: {
|
||||
log: runtimeLog,
|
||||
error: vi.fn(),
|
||||
exit: vi.fn(),
|
||||
},
|
||||
});
|
||||
await stopSlackMonitor(monitor);
|
||||
|
||||
expect(createSlackStartupAuthClientMock).toHaveBeenCalledWith(
|
||||
"bot-token",
|
||||
expect.objectContaining({
|
||||
agent: expect.anything(),
|
||||
slackApiUrl: "https://slack.test/api/",
|
||||
}),
|
||||
);
|
||||
expect(getSlackClient().auth.test).toHaveBeenCalledTimes(1);
|
||||
expect(appStartMock).toHaveBeenCalledTimes(1);
|
||||
expect(runtimeLog).toHaveBeenCalledWith(expect.stringContaining("timeout of 10000ms exceeded"));
|
||||
});
|
||||
|
||||
it("settles and closes a real stalled startup auth request before degraded startup", async () => {
|
||||
const events: string[] = [];
|
||||
for (const key of PROXY_ENV_KEYS) {
|
||||
vi.stubEnv(key, "");
|
||||
}
|
||||
const server = await startStalledSlackApiServer(events);
|
||||
vi.stubEnv("SLACK_API_URL", server.apiUrl);
|
||||
useRealSlackStartupAuthClientOnce();
|
||||
|
||||
const runtimeLog = vi.fn((...args: unknown[]) => {
|
||||
const message = args[0];
|
||||
if (typeof message === "string" && message.includes("slack auth.test failed at boot")) {
|
||||
events.push("auth-settled");
|
||||
}
|
||||
});
|
||||
const { appStartMock } = getSlackTestState();
|
||||
appStartMock.mockImplementationOnce(async () => {
|
||||
events.push("app-start");
|
||||
});
|
||||
const monitor = startSlackMonitor(monitorSlackProvider, {
|
||||
runtime: { log: runtimeLog, error: vi.fn(), exit: vi.fn() },
|
||||
});
|
||||
try {
|
||||
await vi.waitFor(() => expect(appStartMock).toHaveBeenCalledTimes(1), { timeout: 12_000 });
|
||||
await vi.waitFor(() => expect(events).toContain("socket-closed"), { timeout: 1_000 });
|
||||
|
||||
expect(server.requestCount).toBe(1);
|
||||
expect(server.requestUrl).toBe("/api/auth.test");
|
||||
expect(events).toContain("auth-settled");
|
||||
expect(events.indexOf("auth-settled")).toBeLessThan(events.indexOf("app-start"));
|
||||
expect(runtimeLog).toHaveBeenCalledWith(
|
||||
expect.stringContaining("timeout of 10000ms exceeded"),
|
||||
);
|
||||
} finally {
|
||||
monitor.controller.abort();
|
||||
await monitor.run;
|
||||
await server.close();
|
||||
}
|
||||
}, 20_000);
|
||||
|
||||
it("preserves workspace startup when auth.test omits app_id", async () => {
|
||||
getSlackClient().auth.test.mockResolvedValueOnce({
|
||||
user_id: "UBOT",
|
||||
|
||||
@@ -34,6 +34,7 @@ import {
|
||||
} from "../accounts.js";
|
||||
import { isSlackAnyNativeApprovalClientEnabled } from "../approval-native-gates.js";
|
||||
import { resolveSlackWebClientOptions } from "../client-options.js";
|
||||
import { createSlackStartupAuthClient } from "../client.js";
|
||||
import { normalizeSlackWebhookPath, registerSlackHttpHandler } from "../http/index.js";
|
||||
import { SLACK_TEXT_LIMIT } from "../limits.js";
|
||||
import { resolveSlackChannelAllowlist } from "../resolve-channels.js";
|
||||
@@ -379,12 +380,11 @@ export async function monitorSlackProvider(opts: MonitorSlackOpts = {}) {
|
||||
let botId = "";
|
||||
const expectedApiAppIdFromAppToken =
|
||||
slackMode === "socket" ? parseApiAppIdFromAppToken(appToken) : undefined;
|
||||
let authTestFailed = false;
|
||||
let authTestError: string | undefined;
|
||||
let authIdentityWarning: string | undefined;
|
||||
let authTestIdentity: SlackAuthTestIdentity | undefined;
|
||||
try {
|
||||
const auth = await app.client.auth.test();
|
||||
const auth = await createSlackStartupAuthClient(botToken, clientOptions).auth.test();
|
||||
const authUserId = normalizeOptionalString(auth.user_id) ?? "";
|
||||
botId = normalizeOptionalString((auth as { bot_id?: string }).bot_id) ?? "";
|
||||
// Slack documents bot_id only for bot-token identities. Never treat the user behind a
|
||||
@@ -396,26 +396,24 @@ export async function monitorSlackProvider(opts: MonitorSlackOpts = {}) {
|
||||
accountId: account.accountId,
|
||||
});
|
||||
if (!authUserId && !enterpriseOrgInstall) {
|
||||
authTestFailed = true;
|
||||
authTestError = "auth.test returned no user_id";
|
||||
}
|
||||
} catch (err) {
|
||||
authTestFailed = true;
|
||||
authTestError = err instanceof Error ? err.message : String(err);
|
||||
}
|
||||
const installationIdentity = resolveSlackInstallationIdentity({
|
||||
enterpriseOrgInstall,
|
||||
auth: authTestFailed ? undefined : authTestIdentity,
|
||||
auth: authTestError === undefined ? authTestIdentity : undefined,
|
||||
authError: authTestError,
|
||||
transportApiAppId: expectedApiAppIdFromAppToken,
|
||||
});
|
||||
const teamId = installationIdentity.kind === "workspace" ? installationIdentity.teamId : "";
|
||||
const apiAppId =
|
||||
installationIdentity.kind === "degraded" ? "" : (installationIdentity.apiAppId ?? "");
|
||||
if (authTestFailed) {
|
||||
if (authTestError !== undefined) {
|
||||
runtime.log?.(
|
||||
warn(
|
||||
`[${account.accountId}] slack auth.test failed at boot (${authTestError ?? "unknown error"}); ` +
|
||||
`[${account.accountId}] slack auth.test failed at boot (${authTestError}); ` +
|
||||
"explicit bot-mention detection will be disabled until restart with a valid bot token; " +
|
||||
"required-mention channels will fail closed without another trusted activation signal",
|
||||
),
|
||||
|
||||
Reference in New Issue
Block a user