mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-06 08:00:42 +00:00
fix(slack): treat Slack Connect finalize errors as benign in stopSlackStream
When Slack's chat.stopStream fails with user_not_found (Slack Connect DM recipients), team_not_found (cross-workspace shared channels), or missing_recipient_user_id (DM closed mid-stream), the text already delivered via append() is still visible to the user. Swallow those specific codes and mark the session stopped rather than surfacing a spurious 'slack-stream: failed to stop stream' error in dispatch. Other Slack API errors still propagate. Fixes #70295
This commit is contained in:
committed by
Peter Steinberger
parent
688fc288af
commit
676ed34cbd
76
extensions/slack/src/streaming.test.ts
Normal file
76
extensions/slack/src/streaming.test.ts
Normal file
@@ -0,0 +1,76 @@
|
||||
import type { ChatStreamer } from "@slack/web-api/dist/chat-stream.js";
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
import { stopSlackStream, type SlackStreamSession } from "./streaming.js";
|
||||
|
||||
function makeSession(stopImpl: () => Promise<void>): SlackStreamSession {
|
||||
return {
|
||||
streamer: {
|
||||
append: vi.fn(async () => {}),
|
||||
stop: vi.fn(stopImpl),
|
||||
} as unknown as ChatStreamer,
|
||||
channel: "C123",
|
||||
threadTs: "1700000000.000100",
|
||||
stopped: false,
|
||||
};
|
||||
}
|
||||
|
||||
function slackApiError(code: string): Error {
|
||||
const err = new Error(`An API error occurred: ${code}`);
|
||||
(err as unknown as { data: { error: string } }).data = { error: code };
|
||||
return err;
|
||||
}
|
||||
|
||||
describe("stopSlackStream finalize error handling", () => {
|
||||
it("swallows user_not_found (Slack Connect DMs) and marks the session stopped", async () => {
|
||||
const session = makeSession(async () => {
|
||||
throw slackApiError("user_not_found");
|
||||
});
|
||||
await expect(stopSlackStream({ session })).resolves.toBeUndefined();
|
||||
expect(session.stopped).toBe(true);
|
||||
});
|
||||
|
||||
it("swallows team_not_found (Slack Connect cross-workspace) and marks stopped", async () => {
|
||||
const session = makeSession(async () => {
|
||||
throw slackApiError("team_not_found");
|
||||
});
|
||||
await expect(stopSlackStream({ session })).resolves.toBeUndefined();
|
||||
expect(session.stopped).toBe(true);
|
||||
});
|
||||
|
||||
it("swallows missing_recipient_user_id (DM closed mid-stream) and marks stopped", async () => {
|
||||
const session = makeSession(async () => {
|
||||
throw slackApiError("missing_recipient_user_id");
|
||||
});
|
||||
await expect(stopSlackStream({ session })).resolves.toBeUndefined();
|
||||
expect(session.stopped).toBe(true);
|
||||
});
|
||||
|
||||
it("re-throws unexpected Slack API errors so callers can log them", async () => {
|
||||
const session = makeSession(async () => {
|
||||
throw slackApiError("not_authed");
|
||||
});
|
||||
await expect(stopSlackStream({ session })).rejects.toThrow(/not_authed/);
|
||||
// Session is still marked stopped so retries do not re-enter streamer.stop.
|
||||
expect(session.stopped).toBe(true);
|
||||
});
|
||||
|
||||
it("re-throws non-Slack-shaped errors unchanged", async () => {
|
||||
const session = makeSession(async () => {
|
||||
throw new Error("socket reset");
|
||||
});
|
||||
await expect(stopSlackStream({ session })).rejects.toThrow(/socket reset/);
|
||||
expect(session.stopped).toBe(true);
|
||||
});
|
||||
|
||||
it("returns a no-op on an already-stopped session", async () => {
|
||||
const stop = vi.fn(async () => {});
|
||||
const session: SlackStreamSession = {
|
||||
streamer: { append: vi.fn(async () => {}), stop } as unknown as ChatStreamer,
|
||||
channel: "C123",
|
||||
threadTs: "1700000000.000100",
|
||||
stopped: true,
|
||||
};
|
||||
await expect(stopSlackStream({ session })).resolves.toBeUndefined();
|
||||
expect(stop).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
@@ -130,6 +130,12 @@ export async function appendSlackStream(params: AppendSlackStreamParams): Promis
|
||||
*
|
||||
* After calling this the stream message becomes a normal Slack message.
|
||||
* Optionally include final text to append before stopping.
|
||||
*
|
||||
* If Slack's `chat.stopStream` responds with a known benign finalize error
|
||||
* (e.g. `user_not_found` for Slack Connect recipients - see issue #70295),
|
||||
* any text already delivered via `append()` stays visible and the session
|
||||
* is marked stopped. Other Slack API errors still propagate so the caller
|
||||
* can record them.
|
||||
*/
|
||||
export async function stopSlackStream(params: StopSlackStreamParams): Promise<void> {
|
||||
const { session, text } = params;
|
||||
@@ -147,7 +153,73 @@ export async function stopSlackStream(params: StopSlackStreamParams): Promise<vo
|
||||
}`,
|
||||
);
|
||||
|
||||
await session.streamer.stop(text ? { markdown_text: text } : undefined);
|
||||
try {
|
||||
await session.streamer.stop(text ? { markdown_text: text } : undefined);
|
||||
} catch (err) {
|
||||
if (isBenignSlackFinalizeError(err)) {
|
||||
logVerbose(
|
||||
`slack-stream: finalize rejected by Slack (${formatSlackError(err)}); ` +
|
||||
"appended text remains visible, treating stream as stopped",
|
||||
);
|
||||
return;
|
||||
}
|
||||
throw err;
|
||||
}
|
||||
|
||||
logVerbose("slack-stream: stream stopped");
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Finalize error classification
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
/**
|
||||
* Slack API error codes that indicate `chat.stopStream` cannot finalize the
|
||||
* stream for the current recipient/team, but any `chat.appendStream` calls
|
||||
* that already landed are still visible to the user. Treat these as benign
|
||||
* at the dispatch layer so the reply is not reported as an error when text
|
||||
* did get through.
|
||||
*/
|
||||
const BENIGN_SLACK_FINALIZE_ERROR_CODES = new Set<string>([
|
||||
// Slack Connect recipients: finalize fails because the external user id
|
||||
// is not resolvable in the host workspace (#70295).
|
||||
"user_not_found",
|
||||
// Slack Connect team mismatch in shared channels.
|
||||
"team_not_found",
|
||||
// DMs that closed between stream start and stop.
|
||||
"missing_recipient_user_id",
|
||||
]);
|
||||
|
||||
function isBenignSlackFinalizeError(err: unknown): boolean {
|
||||
const code = extractSlackErrorCode(err);
|
||||
return code !== undefined && BENIGN_SLACK_FINALIZE_ERROR_CODES.has(code);
|
||||
}
|
||||
|
||||
function extractSlackErrorCode(err: unknown): string | undefined {
|
||||
if (!err || typeof err !== "object") {
|
||||
return undefined;
|
||||
}
|
||||
const record = err as Record<string, unknown>;
|
||||
// @slack/web-api errors expose `data.error` with the Slack error code.
|
||||
if (record.data && typeof record.data === "object") {
|
||||
const inner = (record.data as Record<string, unknown>).error;
|
||||
if (typeof inner === "string") {
|
||||
return inner;
|
||||
}
|
||||
}
|
||||
// Fallback: parse from message string ("An API error occurred: user_not_found").
|
||||
const message = typeof record.message === "string" ? record.message : "";
|
||||
const match = message.match(/An API error occurred:\s*([a-z_][a-z0-9_]*)/i);
|
||||
return match?.[1];
|
||||
}
|
||||
|
||||
function formatSlackError(err: unknown): string {
|
||||
const code = extractSlackErrorCode(err);
|
||||
if (code) {
|
||||
return code;
|
||||
}
|
||||
if (err instanceof Error) {
|
||||
return err.message;
|
||||
}
|
||||
return String(err);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user