fix(slack): surface silent errors in thread starter/history fetch

martingarramon · steipete · commit 31e5cd6376ee · 2026-04-18T17:55:05.000+01:00
Fixes #62571. `resolveSlackThreadStarter` and `resolveSlackThreadHistory` in `extensions/slack/src/monitor/media.ts` swallowed ALL errors with bare `catch {}` blocks — auth failures, rate-limit rejections, scope errors, and network blips all mapped to the same silent `null` / `[]` fallback. Operators had no way to distinguish "genuinely empty thread" from "Slack rejected our call". Replaces both bare catches with `logVerbose` calls that include the channel, thread ts, and error message. Behavior is preserved — callers still receive `null` / `[]` — but the failure reason now shows up in verbose logs, matching the pattern already used elsewhere in the Slack extension (see `monitor/context.ts:285`, `send.ts:140`, `actions.ts:49`). Testing: - New `describe("resolveSlackThreadStarter", ...)` block with 4 tests (previously uncovered): success path, empty-text skip, Error throw surfaces via logVerbose with channel/ts/reason, non-Error throw value surfaces via String(err). - Existing `resolveSlackThreadHistory` throws test upgraded to assert the logVerbose call with channel/ts/reason. - `pnpm vitest run extensions/slack/src/monitor/media.test.ts` → 35 passed (31 previous + 4 new).
diff --git a/extensions/slack/src/monitor/media.test.ts b/extensions/slack/src/monitor/media.test.ts
@@ -2,6 +2,7 @@ import * as ssrf from "openclaw/plugin-sdk/infra-runtime";
 import * as mediaFetch from "openclaw/plugin-sdk/media-runtime";
 import type { SavedMedia } from "openclaw/plugin-sdk/media-runtime";
 import * as mediaStore from "openclaw/plugin-sdk/media-runtime";
+import { logVerbose } from "openclaw/plugin-sdk/runtime-env";
 import { type FetchMock, withFetchPreconnect } from "openclaw/plugin-sdk/testing";
 import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
 import { mockPinnedHostnameResolution } from "../../../../src/test-helpers/ssrf.js";
@@ -10,8 +11,16 @@ import {
   resolveSlackAttachmentContent,
   resolveSlackMedia,
   resolveSlackThreadHistory,
+  resolveSlackThreadStarter,
+  resetSlackThreadStarterCacheForTest,
 } from "./media.js";
 
+vi.mock("openclaw/plugin-sdk/runtime-env", () => ({
+  logVerbose: vi.fn(),
+  danger: (message: string) => message,
+  shouldLogVerbose: () => false,
+}));
+
 // Store original fetch
 const originalFetch = globalThis.fetch;
 let mockFetch: ReturnType<typeof vi.fn<FetchMock>>;
@@ -843,7 +852,8 @@ describe("resolveSlackThreadHistory", () => {
     expect(replies).not.toHaveBeenCalled();
   });
 
-  it("returns empty when Slack API throws", async () => {
+  it("returns empty and surfaces the error via logVerbose when Slack API throws", async () => {
+    vi.mocked(logVerbose).mockClear();
     const replies = vi.fn().mockRejectedValueOnce(new Error("slack down"));
     const client = {
       conversations: { replies },
@@ -857,5 +867,88 @@ describe("resolveSlackThreadHistory", () => {
     });
 
     expect(result).toEqual([]);
+    expect(vi.mocked(logVerbose)).toHaveBeenCalledWith(
+      expect.stringContaining("slack thread history fetch failed"),
+    );
+    expect(vi.mocked(logVerbose)).toHaveBeenCalledWith(expect.stringContaining("slack down"));
+    expect(vi.mocked(logVerbose)).toHaveBeenCalledWith(expect.stringContaining("channel=C1"));
+  });
+});
+
+describe("resolveSlackThreadStarter", () => {
+  beforeEach(() => {
+    resetSlackThreadStarterCacheForTest();
+    vi.mocked(logVerbose).mockClear();
+  });
+
+  it("returns the starter message when the Slack API succeeds", async () => {
+    const replies = vi.fn().mockResolvedValueOnce({
+      messages: [{ text: "hello thread", user: "U1", ts: "1.000" }],
+    });
+    const client = {
+      conversations: { replies },
+    } as unknown as Parameters<typeof resolveSlackThreadStarter>[0]["client"];
+
+    const result = await resolveSlackThreadStarter({
+      channelId: "C1",
+      threadTs: "1.000",
+      client,
+    });
+
+    expect(result).toEqual({ text: "hello thread", userId: "U1", botId: undefined, ts: "1.000", files: undefined });
+    expect(vi.mocked(logVerbose)).not.toHaveBeenCalled();
+  });
+
+  it("returns null when the starter message has no text and no files", async () => {
+    const replies = vi.fn().mockResolvedValueOnce({ messages: [{ text: "   ", user: "U1" }] });
+    const client = {
+      conversations: { replies },
+    } as unknown as Parameters<typeof resolveSlackThreadStarter>[0]["client"];
+
+    const result = await resolveSlackThreadStarter({
+      channelId: "C1",
+      threadTs: "1.000",
+      client,
+    });
+
+    expect(result).toBeNull();
+    expect(vi.mocked(logVerbose)).not.toHaveBeenCalled();
+  });
+
+  it("returns null and surfaces the error via logVerbose when Slack API throws", async () => {
+    const replies = vi.fn().mockRejectedValueOnce(new Error("not_in_channel"));
+    const client = {
+      conversations: { replies },
+    } as unknown as Parameters<typeof resolveSlackThreadStarter>[0]["client"];
+
+    const result = await resolveSlackThreadStarter({
+      channelId: "C42",
+      threadTs: "9.999",
+      client,
+    });
+
+    expect(result).toBeNull();
+    expect(vi.mocked(logVerbose)).toHaveBeenCalledWith(
+      expect.stringContaining("slack thread starter fetch failed"),
+    );
+    expect(vi.mocked(logVerbose)).toHaveBeenCalledWith(expect.stringContaining("not_in_channel"));
+    expect(vi.mocked(logVerbose)).toHaveBeenCalledWith(expect.stringContaining("channel=C42"));
+    expect(vi.mocked(logVerbose)).toHaveBeenCalledWith(expect.stringContaining("ts=9.999"));
+  });
+
+  it("surfaces non-Error thrown values as String(err) via logVerbose", async () => {
+    const replies = vi.fn().mockRejectedValueOnce("rate_limited");
+    const client = {
+      conversations: { replies },
+    } as unknown as Parameters<typeof resolveSlackThreadStarter>[0]["client"];
+
+    const result = await resolveSlackThreadStarter({
+      channelId: "C1",
+      threadTs: "1.000",
+      client,
+    });
+
+    expect(result).toBeNull();
+    expect(vi.mocked(logVerbose)).toHaveBeenCalledWith(expect.stringContaining("rate_limited"));
   });
 });
diff --git a/extensions/slack/src/monitor/media.ts b/extensions/slack/src/monitor/media.ts
@@ -5,6 +5,7 @@ import type { FetchLike } from "openclaw/plugin-sdk/media-runtime";
 import { fetchRemoteMedia } from "openclaw/plugin-sdk/media-runtime";
 import { saveMediaBuffer } from "openclaw/plugin-sdk/media-runtime";
 import { resolveRequestUrl } from "openclaw/plugin-sdk/request-url";
+import { logVerbose } from "openclaw/plugin-sdk/runtime-env";
 import {
   normalizeLowercaseStringOrEmpty,
   normalizeOptionalLowercaseString,
@@ -447,7 +448,10 @@ export async function resolveSlackThreadStarter(params: {
     });
     evictThreadStarterCache();
     return starter;
-  } catch {
+  } catch (err) {
+    logVerbose(
+      `slack thread starter fetch failed channel=${params.channelId} ts=${params.threadTs}: ${err instanceof Error ? err.message : String(err)}`,
+    );
     return null;
   }
 }
@@ -539,7 +543,10 @@ export async function resolveSlackThreadHistory(params: {
       ts: msg.ts,
       files: msg.files,
     }));
-  } catch {
+  } catch (err) {
+    logVerbose(
+      `slack thread history fetch failed channel=${params.channelId} ts=${params.threadTs}: ${err instanceof Error ? err.message : String(err)}`,
+    );
     return [];
   }
 }
