Skip to content

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

Merged
steipete merged 3 commits into
openclaw:mainfrom
martingarramon:fix/slack-thread-history-surface-errors
Apr 18, 2026
Merged

fix(slack): surface silent errors in thread starter/history fetch#68594
steipete merged 3 commits into
openclaw:mainfrom
martingarramon:fix/slack-thread-history-surface-errors

Conversation

@martingarramon

Copy link
Copy Markdown
Contributor

Fixes #62571.

Problem

resolveSlackThreadStarter and resolveSlackThreadHistory in extensions/slack/src/monitor/media.ts swallow ALL errors via bare catch {} blocks at lines 450–452 and 542–544 respectively. Auth failures, rate-limit rejections, scope errors (not_in_channel, channel_not_found), and network blips all map to the same silent null / [] fallback. Operators have no way to distinguish "genuinely empty thread" from "Slack rejected our call", and thread-context hydration can degrade silently in production.

Why it matters

Thread starter/history hydration is called at the front of every new Slack thread session to populate context. When it silently returns null / [], the agent proceeds without the missing context and the operator sees no log line explaining why. The fix preserves that best-effort behavior but surfaces the reason.

What changed

  1. extensions/slack/src/monitor/media.ts — imports logVerbose from openclaw/plugin-sdk/runtime-env and replaces both bare catch {} blocks with catch (err) { logVerbose(\... channel=${params.channelId} ts=${params.threadTs}: ${err}`); return null/[]; }. Matches the established pattern already used elsewhere in this extension (monitor/context.ts:285, send.ts:140, actions.ts:49`).
  2. extensions/slack/src/monitor/media.test.ts — mocks logVerbose via vi.mock("openclaw/plugin-sdk/runtime-env", ...). Upgrades the existing resolveSlackThreadHistory throws test to assert the log call with channel/ts/reason. Adds a new describe("resolveSlackThreadStarter", ...) block with 4 tests (the function had no direct coverage): success path, empty-text skip, Error throw surfaces via logVerbose with channel + ts + reason, non-Error thrown value surfaces via String(err).

No behavior change for callers — they still receive null / [] on failure. The log is verbose-level, so no noise increase for normal operation.

Testing

  • pnpm vitest run extensions/slack/src/monitor/media.test.ts35 passed (31 previous + 4 new).

What did NOT change (scope boundary)

  • No change to the Slack client, the thread-starter cache, or the happy-path code in either resolver.
  • No change to log level — stays at logVerbose (silent unless OPENCLAW_DEBUG is on), matching sibling slack logging.
  • Error classification (permanent vs. transient) is deliberately NOT added here — that's a separate concern tracked on Slack message tool: agent loops on message_not_found / fileId errors, burns API quota #37544. This PR just makes the failure visible.

Root Cause

  • Root cause: bare catch {} blocks added when the resolvers were first written, never updated as the Slack extension grew logVerbose for error-surface diagnostics elsewhere.
  • Missing detection / guardrail: none — tests only asserted the fallback return value, never the log behavior.

Regression Test Plan

  • Coverage level that should have caught this: Unit test
  • Target test file: extensions/slack/src/monitor/media.test.ts
  • Scenario the test locks in: when conversations.replies throws, logVerbose is called with channel + ts + reason, and the resolver still returns the fallback.

@openclaw-barnacle openclaw-barnacle Bot added channel: slack Channel integration: slack size: S labels Apr 18, 2026
@greptile-apps

greptile-apps Bot commented Apr 18, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

Replaces two bare catch {} blocks in resolveSlackThreadStarter and resolveSlackThreadHistory with catch (err) blocks that emit a logVerbose message (channel, threadTs, and error reason) before returning the existing null/[] fallback. The pattern and import match how errors are already surfaced elsewhere in the Slack extension (context.ts, send.ts, actions.ts).

Confidence Score: 5/5

Safe to merge — minimal targeted change with no behavior impact for callers.

Both changes are one-liner catch-block upgrades that preserve existing fallback return values. The logVerbose import and call site follow the established Slack extension pattern. Tests cover success, empty-text skip, Error throw, non-Error throw, and the channel/ts/reason content of the log message. No P0 or P1 findings.

No files require special attention.

Reviews (1): Last reviewed commit: "fix(slack): surface silent errors in thr..." | Re-trigger Greptile

@openclaw-barnacle openclaw-barnacle Bot added the scripts Repository scripts label Apr 18, 2026
@steipete
steipete force-pushed the fix/slack-thread-history-surface-errors branch from a2ce7ea to f9d6d22 Compare April 18, 2026 16:46
martingarramon and others added 3 commits April 18, 2026 17:50
Fixes openclaw#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).
The `lint:tmp:no-raw-channel-fetch` allowlist pins exact line numbers
(scripts/check-no-raw-channel-fetch.mjs:63-65). The previous commit
added `import { logVerbose } from "openclaw/plugin-sdk/runtime-env";`
on line 8 of `extensions/slack/src/monitor/media.ts`, shifting the
three allowlisted raw `fetch()` callsites from 96/115/120 → 97/116/121.
Updates the allowlist to match the new positions. No behavior change —
the same callsites remain allowlisted.
@steipete
steipete force-pushed the fix/slack-thread-history-surface-errors branch from f9d6d22 to 8b776ad Compare April 18, 2026 16:51
@steipete
steipete merged commit aad9a83 into openclaw:main Apr 18, 2026
47 checks passed
@steipete

Copy link
Copy Markdown
Contributor

Landed via rebase. Thanks @martingarramon.

Main commits:

Validation:

  • CI green on refreshed head.
  • Local: pnpm check, pnpm build, Slack scoped tests, format check, raw channel fetch guard.

martingarramon added a commit to martingarramon/openclaw that referenced this pull request Apr 19, 2026
Adds 5 vitest cases for postSlackMessageBestEffort's silent retry
behavior when Slack rejects a chat:write.customize-identity post:

- Retry on err.data.needed matching chat:write.customize
- Retry on chat:write.customize in response_metadata.acceptedScopes
- Retry on chat:write.customize in response_metadata.scopes
- Rethrow on different missing_scope (e.g. channels:history)
- Rethrow when identity is empty (hasCustomIdentity returns false)

Verifies call count + logVerbose emission in each case.

Follow-up to openclaw#68594.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Mquarmoc pushed a commit to Mquarmoc/openclaw that referenced this pull request Apr 20, 2026
medikoo pushed a commit to medikoo/openclaw that referenced this pull request Apr 22, 2026
medikoo pushed a commit to medikoo/openclaw that referenced this pull request Apr 24, 2026
lovewanwan pushed a commit to lovewanwan/openclaw that referenced this pull request Apr 28, 2026
lovewanwan pushed a commit to lovewanwan/openclaw that referenced this pull request Apr 28, 2026
ogt-redknie pushed a commit to ogt-redknie/OPENX that referenced this pull request May 2, 2026
ogt-redknie pushed a commit to ogt-redknie/OPENX that referenced this pull request May 2, 2026
github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request May 9, 2026
github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request May 9, 2026
globalcaos pushed a commit to globalcaos/tinkerclaw that referenced this pull request May 13, 2026
globalcaos pushed a commit to globalcaos/tinkerclaw that referenced this pull request May 13, 2026
github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request May 24, 2026
github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request May 24, 2026
jameslcowan pushed a commit to jameslcowan/openclaw that referenced this pull request Jun 2, 2026
jameslcowan pushed a commit to jameslcowan/openclaw that referenced this pull request Jun 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

channel: slack Channel integration: slack scripts Repository scripts size: S

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Silent error swallowing in Slack thread starter/history fetch

2 participants