fix(slack): treat Slack Connect finalize errors as benign in stopSlackStream

mvanhorn · steipete · commit 676ed34cbd0b · 2026-04-23T02:42:48.000+01:00
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
diff --git a/extensions/slack/src/streaming.test.ts b/extensions/slack/src/streaming.test.ts
@@ -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();
+  });
+});
diff --git a/extensions/slack/src/streaming.ts b/extensions/slack/src/streaming.ts
@@ -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);
+}
