mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-12 21:53:00 -06:00
fix(slack): retry transient member auth lookups (#122356)
This commit is contained in:
@@ -1,3 +1,4 @@
|
||||
import { WebAPIPlatformError, WebAPIRequestError } from "@slack/web-api";
|
||||
import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import type { SlackMonitorContext } from "./context.js";
|
||||
|
||||
@@ -6,6 +7,7 @@ let authorizeSlackBotRoomMessage: typeof import("./auth.js").authorizeSlackBotRo
|
||||
let authorizeSlackSystemEventSender: typeof import("./auth.js").authorizeSlackSystemEventSender;
|
||||
let resolveSlackEffectiveAllowFrom: typeof import("./auth.js").resolveSlackEffectiveAllowFrom;
|
||||
let resolveSlackCommandIngress: typeof import("./auth.js").resolveSlackCommandIngress;
|
||||
let SlackSystemEventAuthRetryError: typeof import("./auth.js").SlackSystemEventAuthRetryError;
|
||||
|
||||
beforeAll(async () => {
|
||||
({
|
||||
@@ -13,6 +15,7 @@ beforeAll(async () => {
|
||||
authorizeSlackSystemEventSender,
|
||||
resolveSlackCommandIngress,
|
||||
resolveSlackEffectiveAllowFrom,
|
||||
SlackSystemEventAuthRetryError,
|
||||
} = await import("./auth.js"));
|
||||
});
|
||||
|
||||
@@ -47,9 +50,11 @@ function makeSlackCtx(allowFrom: string[]): SlackMonitorContext {
|
||||
|
||||
function makeAuthorizeCtx(params?: {
|
||||
allowFrom?: string[];
|
||||
allowNameMatching?: boolean;
|
||||
channelsConfig?: Record<string, { users?: string[] }>;
|
||||
dmPolicy?: SlackMonitorContext["dmPolicy"];
|
||||
resolveUserName?: (userId: string) => Promise<{ name?: string }>;
|
||||
isChannelAllowed?: () => boolean;
|
||||
resolveUserName?: (userId: string) => Promise<{ name?: string; error?: unknown }>;
|
||||
resolveChannelName?: (
|
||||
channelId: string,
|
||||
) => Promise<{ name?: string; type?: "im" | "mpim" | "channel" | "group" }>;
|
||||
@@ -60,7 +65,7 @@ function makeAuthorizeCtx(params?: {
|
||||
accountId: "main",
|
||||
dmPolicy: params?.dmPolicy ?? "open",
|
||||
dmEnabled: true,
|
||||
allowNameMatching: false,
|
||||
allowNameMatching: params?.allowNameMatching ?? false,
|
||||
channelsConfig: params?.channelsConfig ?? {},
|
||||
channelsConfigKeys: Object.keys(params?.channelsConfig ?? {}),
|
||||
defaultRequireMention: true,
|
||||
@@ -68,7 +73,7 @@ function makeAuthorizeCtx(params?: {
|
||||
kind: "workspace",
|
||||
teamId: "T_MAIN",
|
||||
},
|
||||
isChannelAllowed: vi.fn(() => true),
|
||||
isChannelAllowed: vi.fn(params?.isChannelAllowed ?? (() => true)),
|
||||
resolveUserName: vi.fn(
|
||||
params?.resolveUserName ?? ((_) => Promise.resolve({ name: undefined })),
|
||||
),
|
||||
@@ -100,6 +105,7 @@ const deniedChannel: AuthorizeExpected = {
|
||||
channelName: "general",
|
||||
};
|
||||
const channelUsers = { C1: { users: ["U_ALLOWED"] } };
|
||||
const resolveUserNameError = (error: unknown) => async () => ({ error });
|
||||
|
||||
function interactiveRequest(
|
||||
senderId: string,
|
||||
@@ -220,6 +226,68 @@ describe("resolveSlackEffectiveAllowFrom", () => {
|
||||
});
|
||||
|
||||
describe("authorizeSlackSystemEventSender", () => {
|
||||
it("checks the channel gate and stable ID before resolving a member name", async () => {
|
||||
const deniedCtx = makeAuthorizeCtx({
|
||||
allowNameMatching: true,
|
||||
channelsConfig: { C1: { users: ["alice"] } },
|
||||
isChannelAllowed: () => false,
|
||||
});
|
||||
await expect(
|
||||
authorizeSlackSystemEventSender({
|
||||
ctx: deniedCtx,
|
||||
senderId: "U_DENIED",
|
||||
channelId: "C1",
|
||||
retryNameLookup: true,
|
||||
}),
|
||||
).resolves.toMatchObject({ allowed: false, reason: "channel-not-allowed" });
|
||||
expect(deniedCtx.resolveUserName).not.toHaveBeenCalled();
|
||||
|
||||
const allowedCtx = makeAuthorizeCtx({
|
||||
allowNameMatching: true,
|
||||
channelsConfig: channelUsers,
|
||||
});
|
||||
await expect(
|
||||
authorizeSlackSystemEventSender({
|
||||
ctx: allowedCtx,
|
||||
senderId: "U_ALLOWED",
|
||||
channelId: "C1",
|
||||
retryNameLookup: true,
|
||||
}),
|
||||
).resolves.toEqual(allowedChannel);
|
||||
expect(allowedCtx.resolveUserName).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("retries only transient direct-name lookup failures", async () => {
|
||||
const authorize = (error: unknown) =>
|
||||
authorizeSlackSystemEventSender({
|
||||
ctx: makeAuthorizeCtx({
|
||||
allowNameMatching: true,
|
||||
channelsConfig: { C1: { users: ["alice"] } },
|
||||
resolveUserName: resolveUserNameError(error),
|
||||
}),
|
||||
senderId: "U_PENDING",
|
||||
channelId: "C1",
|
||||
retryNameLookup: true,
|
||||
});
|
||||
for (const error of [
|
||||
new WebAPIRequestError(Object.assign(new Error("socket reset"), { code: "ECONNRESET" })),
|
||||
new WebAPIPlatformError({ ok: false, error: "service_unavailable" }),
|
||||
]) {
|
||||
await expect(authorize(error)).rejects.toBeInstanceOf(SlackSystemEventAuthRetryError);
|
||||
}
|
||||
|
||||
for (const error of [
|
||||
new WebAPIPlatformError({ ok: false, error: "user_not_found" }),
|
||||
new WebAPIRequestError(new DOMException("request was canceled", "AbortError")),
|
||||
new TypeError("invalid URL"),
|
||||
]) {
|
||||
await expect(authorize(error)).resolves.toMatchObject({
|
||||
allowed: false,
|
||||
reason: "sender-not-channel-allowed",
|
||||
});
|
||||
}
|
||||
});
|
||||
|
||||
it.each([
|
||||
[
|
||||
"ignores non-decimal channel member cache ttl env values",
|
||||
|
||||
@@ -4,7 +4,6 @@ import {
|
||||
type ChannelIngressIdentifierKind,
|
||||
type ChannelIngressPolicyInput,
|
||||
type ChannelIngressStateInput,
|
||||
type ChannelIngressDecision,
|
||||
createChannelIngressResolver,
|
||||
defineStableChannelIngressIdentity,
|
||||
readChannelIngressStoreAllowFromForDmPolicy,
|
||||
@@ -28,6 +27,7 @@ import { resolveSlackChannelConfig } from "./channel-config.js";
|
||||
import { inferSlackChannelType } from "./channel-type.js";
|
||||
import { normalizeSlackChannelType, type SlackMonitorContext } from "./context.js";
|
||||
import type { SlackEventScope } from "./event-scope.js";
|
||||
import { isTransientSlackThreadLookupError } from "./thread-resolution.js";
|
||||
|
||||
type SlackChannelMembersCacheEntry = {
|
||||
expiresAtMs: number;
|
||||
@@ -36,18 +36,8 @@ type SlackChannelMembersCacheEntry = {
|
||||
};
|
||||
|
||||
type SlackIngressChannelType = "im" | "mpim" | "channel" | "group";
|
||||
type SlackSystemEventAuthorization =
|
||||
| {
|
||||
allowed: true;
|
||||
channelType?: SlackIngressChannelType;
|
||||
channelName?: string;
|
||||
}
|
||||
| {
|
||||
allowed: false;
|
||||
reason: string;
|
||||
channelType?: SlackIngressChannelType;
|
||||
channelName?: string;
|
||||
};
|
||||
type SlackSystemEventAuthorization = ({ allowed: true } | { allowed: false; reason: string }) &
|
||||
Partial<{ channelType: SlackIngressChannelType; channelName: string }>;
|
||||
|
||||
const slackChannelMembersCache = new WeakMap<
|
||||
SlackMonitorContext,
|
||||
@@ -59,6 +49,7 @@ const SLACK_CHANNEL_ID = "slack";
|
||||
const SLACK_USER_NAME_KIND =
|
||||
"plugin:slack-user-name" as const satisfies ChannelIngressIdentifierKind;
|
||||
|
||||
export class SlackSystemEventAuthRetryError extends Error {}
|
||||
function normalizeSlackUserId(raw?: string | null): string {
|
||||
const value = (raw ?? "").trim().toLowerCase();
|
||||
if (!value) {
|
||||
@@ -477,7 +468,9 @@ async function decideSlackSystemIngress(params: {
|
||||
ownerAllowFromLower: string[];
|
||||
channelUsers?: Array<string | number>;
|
||||
interactiveEvent: boolean;
|
||||
}): Promise<ChannelIngressDecision> {
|
||||
retryNameLookup?: boolean;
|
||||
eventScope?: SlackEventScope;
|
||||
}) {
|
||||
const isDirectMessage = params.channelType === "im";
|
||||
const isGroupDm = params.channelType === "mpim";
|
||||
const teamId = params.teamId ?? params.ctx.teamId;
|
||||
@@ -514,13 +507,16 @@ async function decideSlackSystemIngress(params: {
|
||||
}
|
||||
return params.channelId ? ["*"] : wildcardWhenOpen(ownerAllowFromLower);
|
||||
})();
|
||||
const result = await createSlackIngressResolver(params.ctx).message({
|
||||
subject: createSlackIngressSubject({
|
||||
const subject = (senderName?: string) =>
|
||||
createSlackIngressSubject({
|
||||
senderId: params.senderId,
|
||||
senderName: params.senderName,
|
||||
senderName,
|
||||
teamId,
|
||||
workspaceScoped: !allowUnscoped,
|
||||
}),
|
||||
});
|
||||
const resolver = createSlackIngressResolver(params.ctx);
|
||||
const input: Parameters<typeof resolver.message>[0] = {
|
||||
subject: subject(params.senderName),
|
||||
conversation: {
|
||||
kind: slackIngressConversationKind(params.channelType),
|
||||
id: params.channelId ?? "slack-system",
|
||||
@@ -553,7 +549,23 @@ async function decideSlackSystemIngress(params: {
|
||||
commandOwnerAllowFrom: ownerAllowFrom,
|
||||
}
|
||||
: undefined,
|
||||
});
|
||||
};
|
||||
const result = await resolver.message(input);
|
||||
if (
|
||||
result.ingress.decision !== "allow" &&
|
||||
params.retryNameLookup &&
|
||||
result.state.allowlists[isDirectMessage ? "dm" : "group"].normalizedEntries.some(
|
||||
(entry) => entry.kind === SLACK_USER_NAME_KIND,
|
||||
)
|
||||
) {
|
||||
const lookup = await params.ctx.resolveUserName(params.senderId, params.eventScope);
|
||||
if (lookup.error && isTransientSlackThreadLookupError(lookup.error)) {
|
||||
throw new SlackSystemEventAuthRetryError(formatErrorMessage(lookup.error));
|
||||
}
|
||||
if (lookup.name) {
|
||||
return (await resolver.message({ ...input, subject: subject(lookup.name) })).ingress;
|
||||
}
|
||||
}
|
||||
return result.ingress;
|
||||
}
|
||||
|
||||
@@ -564,6 +576,7 @@ export async function authorizeSlackSystemEventSender(params: {
|
||||
channelType?: string | null;
|
||||
eventScope?: SlackEventScope;
|
||||
expectedSenderId?: string;
|
||||
retryNameLookup?: boolean;
|
||||
/** When true, requires expectedSenderId, rejects ambiguous channel types,
|
||||
* and applies interactive-only owner allowFrom checks without changing the
|
||||
* open-by-default channel behavior when no allowlists are configured. */
|
||||
@@ -637,10 +650,9 @@ export async function authorizeSlackSystemEventSender(params: {
|
||||
}
|
||||
}
|
||||
|
||||
const senderInfo: { name?: string } = await params.ctx
|
||||
.resolveUserName(senderId, params.eventScope)
|
||||
.catch(() => ({}));
|
||||
const senderName = senderInfo.name;
|
||||
const senderInfo = params.retryNameLookup
|
||||
? undefined
|
||||
: await params.ctx.resolveUserName(senderId, params.eventScope);
|
||||
const ingressChannelType = channelType ?? "channel";
|
||||
|
||||
if (ingressChannelType === "im") {
|
||||
@@ -671,12 +683,14 @@ export async function authorizeSlackSystemEventSender(params: {
|
||||
ctx: params.ctx,
|
||||
teamId: params.eventScope?.teamId ?? params.ctx.teamId,
|
||||
senderId,
|
||||
senderName,
|
||||
senderName: senderInfo?.name,
|
||||
channelType: ingressChannelType,
|
||||
channelId,
|
||||
ownerAllowFromLower: allowFromLower,
|
||||
channelUsers: channelConfig?.users,
|
||||
interactiveEvent: params.interactiveEvent === true,
|
||||
retryNameLookup: params.retryNameLookup && params.ctx.allowNameMatching,
|
||||
eventScope: params.eventScope,
|
||||
});
|
||||
if (decision.decision === "allow") {
|
||||
return {
|
||||
|
||||
@@ -59,6 +59,7 @@ type SlackChannelCacheEntry = {
|
||||
metadataLoaded: boolean;
|
||||
};
|
||||
|
||||
type SlackUserInfo = { name?: string; error?: unknown };
|
||||
const SLACK_CHANNEL_CACHE_MAX_ENTRIES = 1024;
|
||||
const SLACK_USER_CACHE_MAX_ENTRIES = 2048;
|
||||
const SLACK_CHANNEL_DENIAL_WARNING_TTL_MS = 5 * 60_000;
|
||||
@@ -137,7 +138,7 @@ export type SlackMonitorContext = {
|
||||
channelId: string | null | undefined,
|
||||
eventScope?: SlackEventScope,
|
||||
) => SlackMessageEvent["channel_type"] | undefined;
|
||||
resolveUserName: (userId: string, eventScope?: SlackEventScope) => Promise<{ name?: string }>;
|
||||
resolveUserName: (userId: string, eventScope?: SlackEventScope) => Promise<SlackUserInfo>;
|
||||
setSlackThreadStatus: (params: {
|
||||
channelId: string;
|
||||
threadTs?: string;
|
||||
@@ -340,8 +341,8 @@ export function createSlackMonitorContext(params: {
|
||||
const entry = { name };
|
||||
writeLruMapEntry(userCache, cacheKey, entry, SLACK_USER_CACHE_MAX_ENTRIES);
|
||||
return entry;
|
||||
} catch {
|
||||
return {};
|
||||
} catch (error) {
|
||||
return { error };
|
||||
}
|
||||
};
|
||||
|
||||
|
||||
@@ -156,6 +156,33 @@ describe("registerSlackMemberEvents", () => {
|
||||
);
|
||||
});
|
||||
|
||||
it("uses the stable user ID when the post-auth name lookup fails", async () => {
|
||||
const harness = initSlackHarness({
|
||||
channelType: "channel",
|
||||
channelUsers: ["U1"],
|
||||
});
|
||||
const resolveUserName = vi.fn(async () => ({ error: new Error("users.info failed") }));
|
||||
harness.ctx.resolveUserName = resolveUserName;
|
||||
registerSlackMemberEvents({ ctx: harness.ctx });
|
||||
const handler = harness.getHandler("member_joined_channel");
|
||||
if (!handler) {
|
||||
throw new Error("expected Slack member joined handler");
|
||||
}
|
||||
|
||||
await handler({
|
||||
event: makeMemberEvent({ channel: "C1", user: "U1" }),
|
||||
body: { event_id: "Ev-member-id-fallback" },
|
||||
});
|
||||
|
||||
expect(resolveUserName).toHaveBeenCalledOnce();
|
||||
expect(memberMocks.enqueue).toHaveBeenCalledWith(
|
||||
"Slack: U1 joined #general.",
|
||||
expect.objectContaining({
|
||||
contextKey: "slack:member:joined:C1:U1:Ev-member-id-fallback",
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("keeps enterprise member events isolated by listener workspace", async () => {
|
||||
const harness = initSlackHarness();
|
||||
harness.ctx.installationIdentity = {
|
||||
|
||||
@@ -3,6 +3,7 @@ import type { AllMiddlewareArgs, SlackEventMiddlewareArgs } from "@slack/bolt";
|
||||
import { formatErrorMessage } from "openclaw/plugin-sdk/error-runtime";
|
||||
import { danger } from "openclaw/plugin-sdk/runtime-env";
|
||||
import { enqueueSystemEvent } from "openclaw/plugin-sdk/system-event-runtime";
|
||||
import { SlackSystemEventAuthRetryError } from "../auth.js";
|
||||
import type { SlackMonitorContext } from "../context.js";
|
||||
import type { SlackMemberChannelEvent } from "../types.js";
|
||||
import {
|
||||
@@ -66,6 +67,9 @@ export function registerSlackMemberEvents(params: {
|
||||
ctx.runtime.error?.(
|
||||
danger(`slack ${paramsLocal.verb} handler failed: ${formatErrorMessage(err)}`),
|
||||
);
|
||||
if (err instanceof SlackSystemEventAuthRetryError) {
|
||||
throw err;
|
||||
}
|
||||
}
|
||||
};
|
||||
|
||||
|
||||
@@ -26,6 +26,7 @@ export async function authorizeAndResolveSlackSystemEventContext(params: {
|
||||
channelId,
|
||||
channelType,
|
||||
eventScope: params.eventScope,
|
||||
retryNameLookup: eventKind.startsWith("member-"),
|
||||
});
|
||||
if (!auth.allowed) {
|
||||
logVerbose(
|
||||
|
||||
@@ -4,19 +4,22 @@ import fs from "node:fs/promises";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
import { App, type Receiver, type ReceiverEvent } from "@slack/bolt";
|
||||
import type { WebClientOptions } from "@slack/web-api";
|
||||
import type { ChannelIngressQueue } from "openclaw/plugin-sdk/channel-outbound";
|
||||
import type { OpenClawConfig } from "openclaw/plugin-sdk/config-contracts";
|
||||
import type { PluginJsonValue } from "openclaw/plugin-sdk/plugin-entry";
|
||||
import {
|
||||
closeOpenClawStateDatabaseForTest,
|
||||
createChannelIngressQueueForTests,
|
||||
} from "openclaw/plugin-sdk/plugin-state-test-runtime";
|
||||
import type { RuntimeEnv } from "openclaw/plugin-sdk/runtime-env";
|
||||
import {
|
||||
peekSystemEventEntries,
|
||||
resetSystemEventsForTest,
|
||||
} from "openclaw/plugin-sdk/system-event-runtime";
|
||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
import { createSlackMonitorContext } from "./context.js";
|
||||
import { registerSlackMemberEvents } from "./events/members.js";
|
||||
import { createSlackSystemEventTestHarness } from "./events/system-event-test-harness.js";
|
||||
import { createSlackDurableIngress, resolveSlackIngressTurnLifecycle } from "./ingress.js";
|
||||
|
||||
type SlackIngressQueue = NonNullable<Parameters<typeof createSlackDurableIngress>[0]["queue"]>;
|
||||
@@ -86,7 +89,11 @@ function createReceiverHarness() {
|
||||
function createReceiverEvent(
|
||||
eventId: string,
|
||||
ack = vi.fn(async () => {}),
|
||||
options: { retryNum?: number; ts?: string; event?: Record<string, PluginJsonValue> } = {},
|
||||
options: {
|
||||
retryNum?: number;
|
||||
ts?: string;
|
||||
event?: Record<string, PluginJsonValue>;
|
||||
} = {},
|
||||
): ReceiverEvent {
|
||||
return {
|
||||
body: createSlackEnvelope(eventId, options.ts, options.event),
|
||||
@@ -108,7 +115,8 @@ function createMemberEvent(type: "member_joined_channel" | "member_left_channel"
|
||||
function attachBoltMemberIngress(params: {
|
||||
queue: ChannelIngressQueue<SlackIngressPayload>;
|
||||
trackEvent: () => void;
|
||||
resolveUserName?: (userId: string) => Promise<{ name?: string }>;
|
||||
usersInfo?: App["client"]["users"]["info"];
|
||||
usersInfoFetch?: NonNullable<WebClientOptions["fetch"]>;
|
||||
pollIntervalMs?: number;
|
||||
}) {
|
||||
const ingress = createSlackDurableIngress({
|
||||
@@ -126,15 +134,73 @@ function attachBoltMemberIngress(params: {
|
||||
botUserId: "U_BOT",
|
||||
teamId: "T_TEST",
|
||||
}),
|
||||
...(params.usersInfoFetch
|
||||
? {
|
||||
clientOptions: {
|
||||
fetch: params.usersInfoFetch,
|
||||
retryConfig: { retries: 0 },
|
||||
slackApiUrl: "https://slack.test/api/",
|
||||
},
|
||||
}
|
||||
: {}),
|
||||
convoStore: false,
|
||||
ignoreSelf: false,
|
||||
});
|
||||
const memberHarness = createSlackSystemEventTestHarness({ channelType: "channel" });
|
||||
memberHarness.ctx.app = app;
|
||||
if (params.resolveUserName) {
|
||||
memberHarness.ctx.resolveUserName = params.resolveUserName;
|
||||
vi.spyOn(app.client.conversations, "info").mockResolvedValue({
|
||||
ok: true,
|
||||
channel: { id: "C_TEST", name: "general", is_channel: true },
|
||||
});
|
||||
if (!params.usersInfoFetch) {
|
||||
vi.spyOn(app.client.users, "info").mockImplementation(
|
||||
params.usersInfo ??
|
||||
(async () => ({
|
||||
ok: true,
|
||||
user: { id: "U_TEST", name: "alice" },
|
||||
})),
|
||||
);
|
||||
}
|
||||
registerSlackMemberEvents({ ctx: memberHarness.ctx, trackEvent: params.trackEvent });
|
||||
const ctx = createSlackMonitorContext({
|
||||
cfg: {} as OpenClawConfig,
|
||||
accountId: "default",
|
||||
botToken: "xoxb-test",
|
||||
app,
|
||||
runtime: {} as RuntimeEnv,
|
||||
botUserId: "U_BOT",
|
||||
botId: "B_BOT",
|
||||
identityHealth: { lifecycle: "ready", lastError: null },
|
||||
teamId: "T_TEST",
|
||||
apiAppId: "A_TEST",
|
||||
installationIdentity: { kind: "workspace", teamId: "T_TEST" },
|
||||
historyLimit: 0,
|
||||
sessionScope: "per-sender",
|
||||
mainKey: "main",
|
||||
dmEnabled: true,
|
||||
dmPolicy: "open",
|
||||
allowFrom: [],
|
||||
allowNameMatching: true,
|
||||
groupDmEnabled: true,
|
||||
groupDmChannels: [],
|
||||
defaultRequireMention: true,
|
||||
channelsConfig: { C_TEST: { users: ["alice"], enabled: true } },
|
||||
groupPolicy: "open",
|
||||
useAccessGroups: false,
|
||||
reactionMode: "off",
|
||||
reactionAllowlist: [],
|
||||
replyToMode: "off",
|
||||
slashCommand: {
|
||||
enabled: false,
|
||||
name: "openclaw",
|
||||
sessionPrefix: "slack:slash",
|
||||
ephemeral: true,
|
||||
},
|
||||
textLimit: 4000,
|
||||
ackReactionScope: "group-mentions",
|
||||
typingReaction: "",
|
||||
mediaMaxBytes: 1,
|
||||
threadHistoryScope: "thread",
|
||||
threadInheritParent: false,
|
||||
});
|
||||
registerSlackMemberEvents({ ctx, trackEvent: params.trackEvent });
|
||||
return { ingress, receive: receiverHarness.receive };
|
||||
}
|
||||
|
||||
@@ -440,7 +506,11 @@ describe("Slack durable ingress", () => {
|
||||
await ingress.waitForIdle();
|
||||
|
||||
expect(trackEvent).toHaveBeenCalledTimes(3);
|
||||
expect(peekSystemEventEntries("agent:main:main").map((entry) => entry.contextKey)).toEqual([
|
||||
expect(
|
||||
peekSystemEventEntries("agent:main:slack:channel:c_test").map(
|
||||
(entry) => entry.contextKey,
|
||||
),
|
||||
).toEqual([
|
||||
"slack:member:joined:c_test:u_test:ev-member-join-1",
|
||||
"slack:member:left:c_test:u_test:ev-member-left",
|
||||
"slack:member:joined:c_test:u_test:ev-member-join-2",
|
||||
@@ -454,15 +524,34 @@ describe("Slack durable ingress", () => {
|
||||
it("retries transient member failures through Bolt after restart", async () => {
|
||||
await withQueue(async (queue) => {
|
||||
const trackEvent = vi.fn();
|
||||
let userLookupCount = 0;
|
||||
const resolveUserName = async () => {
|
||||
userLookupCount += 1;
|
||||
if (userLookupCount === 2) {
|
||||
throw new Error("users.info temporarily unavailable");
|
||||
let usersInfoRequests = 0;
|
||||
const usersInfoFetch = vi.fn<NonNullable<WebClientOptions["fetch"]>>(async (input) => {
|
||||
const pathname = new URL(String(input)).pathname;
|
||||
if (pathname.endsWith("/conversations.info")) {
|
||||
return new Response(
|
||||
JSON.stringify({
|
||||
ok: true,
|
||||
channel: { id: "C_TEST", name: "general", is_channel: true },
|
||||
}),
|
||||
{ headers: { "content-type": "application/json" }, status: 200 },
|
||||
);
|
||||
}
|
||||
return { name: "alice" };
|
||||
};
|
||||
const first = attachBoltMemberIngress({ queue, trackEvent, resolveUserName });
|
||||
if (!pathname.endsWith("/users.info")) {
|
||||
throw new Error(`unexpected Slack API request: ${pathname}`);
|
||||
}
|
||||
usersInfoRequests += 1;
|
||||
if (usersInfoRequests === 1) {
|
||||
return new Response(JSON.stringify({ ok: false, error: "ratelimited" }), {
|
||||
headers: { "content-type": "application/json", "retry-after": "0" },
|
||||
status: 429,
|
||||
});
|
||||
}
|
||||
return new Response(JSON.stringify({ ok: true, user: { id: "U_TEST", name: "alice" } }), {
|
||||
headers: { "content-type": "application/json" },
|
||||
status: 200,
|
||||
});
|
||||
});
|
||||
const first = attachBoltMemberIngress({ queue, trackEvent, usersInfoFetch });
|
||||
first.ingress.start();
|
||||
let restarted: ReturnType<typeof attachBoltMemberIngress> | undefined;
|
||||
try {
|
||||
@@ -475,13 +564,13 @@ describe("Slack durable ingress", () => {
|
||||
await first.ingress.stop();
|
||||
|
||||
expect(trackEvent).toHaveBeenCalledTimes(1);
|
||||
expect(peekSystemEventEntries("agent:main:main")).toHaveLength(0);
|
||||
expect(peekSystemEventEntries("agent:main:slack:channel:c_test")).toHaveLength(0);
|
||||
expect((await queue.listPending()).map((entry) => entry.id)).toContain("Ev-member-retry");
|
||||
|
||||
restarted = attachBoltMemberIngress({
|
||||
queue,
|
||||
trackEvent,
|
||||
resolveUserName,
|
||||
usersInfoFetch,
|
||||
pollIntervalMs: 25,
|
||||
});
|
||||
restarted.ingress.start();
|
||||
@@ -493,7 +582,8 @@ describe("Slack durable ingress", () => {
|
||||
{ timeout: 15_000, interval: 100 },
|
||||
);
|
||||
|
||||
expect(peekSystemEventEntries("agent:main:main")).toHaveLength(1);
|
||||
expect(usersInfoRequests).toBe(2);
|
||||
expect(peekSystemEventEntries("agent:main:slack:channel:c_test")).toHaveLength(1);
|
||||
} finally {
|
||||
await first.ingress.stop();
|
||||
await restarted?.ingress.stop();
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
// Slack tests cover monitor.thread resolution plugin behavior.
|
||||
import {
|
||||
WebClient,
|
||||
WebAPIHTTPError,
|
||||
WebAPIPlatformError,
|
||||
WebAPIRateLimitedError,
|
||||
@@ -8,7 +9,10 @@ import {
|
||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
import type { SlackMessageEvent } from "../types.js";
|
||||
import type { SlackIngressTurnLifecycle } from "./ingress.js";
|
||||
import { createSlackThreadTsResolver } from "./thread-resolution.js";
|
||||
import {
|
||||
createSlackThreadTsResolver,
|
||||
isTransientSlackThreadLookupError,
|
||||
} from "./thread-resolution.js";
|
||||
|
||||
type SlackThreadClient = Parameters<typeof createSlackThreadTsResolver>[0]["client"];
|
||||
|
||||
@@ -80,6 +84,59 @@ describe("createSlackThreadTsResolver", () => {
|
||||
expect(historyMock).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("classifies an exhausted real WebClient 429 as transient", async () => {
|
||||
const fetch = vi.fn(async () => {
|
||||
return new Response(JSON.stringify({ ok: false, error: "ratelimited" }), {
|
||||
headers: { "content-type": "application/json", "retry-after": "0" },
|
||||
status: 429,
|
||||
});
|
||||
});
|
||||
const client = new WebClient("xoxb-test", {
|
||||
fetch,
|
||||
retryConfig: { retries: 0 },
|
||||
slackApiUrl: "https://slack.test/api/",
|
||||
});
|
||||
|
||||
const error: unknown = await client.users
|
||||
.info({ user: "U1" })
|
||||
.catch((caught: unknown) => caught);
|
||||
|
||||
expect(error).toBeInstanceOf(WebAPIRequestError);
|
||||
if (!(error instanceof WebAPIRequestError)) {
|
||||
throw new Error("expected exhausted Slack 429 to become WebAPIRequestError");
|
||||
}
|
||||
expect(error.original.message).toMatch(
|
||||
/^A rate limit was exceeded \(url: .+, retry-after: 0\)$/,
|
||||
);
|
||||
expect(isTransientSlackThreadLookupError(error)).toBe(true);
|
||||
expect(fetch).toHaveBeenCalledOnce();
|
||||
});
|
||||
|
||||
it.each(["internal_error", "service_unavailable"])(
|
||||
"classifies a real WebClient %s platform response as transient",
|
||||
async (code) => {
|
||||
const fetch = vi.fn(async () => {
|
||||
return new Response(JSON.stringify({ ok: false, error: code }), {
|
||||
headers: { "content-type": "application/json" },
|
||||
status: 200,
|
||||
});
|
||||
});
|
||||
const client = new WebClient("xoxb-test", {
|
||||
fetch,
|
||||
retryConfig: { retries: 0 },
|
||||
slackApiUrl: "https://slack.test/api/",
|
||||
});
|
||||
|
||||
const error: unknown = await client.users
|
||||
.info({ user: "U1" })
|
||||
.catch((caught: unknown) => caught);
|
||||
|
||||
expect(error).toBeInstanceOf(WebAPIPlatformError);
|
||||
expect(isTransientSlackThreadLookupError(error)).toBe(true);
|
||||
expect(fetch).toHaveBeenCalledOnce();
|
||||
},
|
||||
);
|
||||
|
||||
it.each([
|
||||
{
|
||||
label: "an actual Slack HTTP 408 timeout",
|
||||
@@ -226,6 +283,10 @@ describe("createSlackThreadTsResolver", () => {
|
||||
label: "operator-canceled Slack request",
|
||||
error: new WebAPIRequestError(new DOMException("request was canceled", "AbortError")),
|
||||
},
|
||||
{
|
||||
label: "uncoded Slack request failure",
|
||||
error: new WebAPIRequestError(new Error("request failed without a transient signal")),
|
||||
},
|
||||
])("preserves cached ambiguity for definitive $label", async ({ error }) => {
|
||||
const historyMock = vi.fn().mockRejectedValue(error);
|
||||
const resolver = createSlackThreadTsResolver({
|
||||
|
||||
@@ -2,6 +2,7 @@
|
||||
import {
|
||||
type WebClient as SlackWebClient,
|
||||
WebAPIHTTPError,
|
||||
WebAPIPlatformError,
|
||||
WebAPIRateLimitedError,
|
||||
WebAPIRequestError,
|
||||
} from "@slack/web-api";
|
||||
@@ -36,7 +37,7 @@ const markAmbiguousThreadReply = (message: SlackMessageEvent): SlackMessageEvent
|
||||
_ambiguousThreadReply: true,
|
||||
});
|
||||
|
||||
function isTransientSlackThreadLookupError(error: unknown): boolean {
|
||||
export function isTransientSlackThreadLookupError(error: unknown): boolean {
|
||||
if (error instanceof WebAPIRateLimitedError) {
|
||||
return true;
|
||||
}
|
||||
@@ -47,9 +48,17 @@ function isTransientSlackThreadLookupError(error: unknown): boolean {
|
||||
(error.statusCode >= 500 && error.statusCode < 600)
|
||||
);
|
||||
}
|
||||
// Slack documents these users.info response codes as transient service failures.
|
||||
if (error instanceof WebAPIPlatformError) {
|
||||
return error.data.error === "internal_error" || error.data.error === "service_unavailable";
|
||||
}
|
||||
if (!(error instanceof WebAPIRequestError)) {
|
||||
return false;
|
||||
}
|
||||
// Slack Web API 8.0.0 wraps exhausted 429 retries as this uncoded request error.
|
||||
if (/^A rate limit was exceeded \(url: .+, retry-after: \d+\)$/.test(error.original.message)) {
|
||||
return true;
|
||||
}
|
||||
return collectErrorGraphCandidates(error.original, (current) => [
|
||||
current.cause,
|
||||
current.error,
|
||||
|
||||
Reference in New Issue
Block a user