Skip to content

fix(web): extend bestEffort to conversation history and settings daemon calls#33163

Closed
devin-ai-integration[bot] wants to merge 1 commit into
mainfrom
devin/1780445015-lum-2199-besteffort-gap
Closed

fix(web): extend bestEffort to conversation history and settings daemon calls#33163
devin-ai-integration[bot] wants to merge 1 commit into
mainfrom
devin/1780445015-lum-2199-besteffort-gap

Conversation

@devin-ai-integration

@devin-ai-integration devin-ai-integration Bot commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

Prompt / plan

Closes LUM-2199

Sentry audit found two remaining daemon call sites reporting expected transient errors:

  • WEB-7 (77 events) — use-conversation-history.ts pagination error handler
  • WEB-2H (1 event) — web-search-card.tsx settings secret read

Both needed bestEffort: true on their captureError calls (introduced in PR #33160). WEB-2H additionally exposed a gap: HeyAPI's throwOnError: true throws the raw JSON response body ({detail: "..."}) as a plain object — not an ApiError instance — so isExpectedDaemonTransientError() didn't match it.

Changes

capture-error.ts — Extend isExpectedDaemonTransientError() to handle raw {detail: string} objects. After the existing ApiError branch, check for plain objects with a detail string and match against known daemon transient substrings (case-insensitive): "still starting up", "organization-id header", "authentication credentials", "bad gateway". This covers the Django REST framework error shape that HeyAPI throws verbatim.

use-conversation-history.ts — Add bestEffort: true to the pagination error captureError call.

web-search-card.tsx — Add bestEffort: true to the secret-read error captureError call.

Why this is safe

  • Additive onlyisExpectedDaemonTransientError gains a new branch after the existing ApiError check; existing ApiError matching is unchanged.
  • Narrow matching — raw object detection requires typeof === "object", non-null, "detail" in error, typeof detail === "string", and substring match against a fixed allowlist. Objects without detail or with non-matching messages pass through to Sentry.
  • No false negatives for real bugs — 500 Internal Server Error, data integrity errors, and programming errors don't contain any of the four allowlisted substrings.

Alternatives not taken

  • Normalize errors before transient-error check — could call normalizeToError() first so isExpectedDaemonTransientError only sees Error instances. Rejected because the normalized Error loses the status code that ApiError carries, making status-based checks impossible. The two-branch approach (ApiError by status, raw object by detail string) preserves both detection paths.
  • Wrap HeyAPI errors at the client interceptor level — could intercept all HeyAPI throwOnError responses and wrap them in ApiError. Rejected because it would change the error shape across every call site in the app, which is a much larger change with higher regression risk.

Root cause analysis

  1. How did the code get into this state?bestEffort mode was introduced in PR fix(web): add bestEffort mode to captureError for daemon transient errors #33160 but only applied to useActiveConversation and useConversationSync. These two call sites were missed because they were in different domains (chat pagination, settings).
  2. What led to it? — No systematic audit of all captureError call sites when bestEffort was introduced.
  3. Warning signs? — WEB-7 had 77 events in Sentry, clearly visible in the project dashboard.
  4. Prevention? — When introducing a new captureError option, grep for all existing captureError calls and evaluate each one.
  5. AGENTS.md update needed? — No. The existing docs/CONVENTIONS.md already documents bestEffort usage. The pattern is clear; this was a coverage gap, not a knowledge gap.

Test plan

  • 33 unit tests pass (bun test src/lib/sentry/capture-error.test.ts) — including 10 new tests for raw {detail} object detection and bestEffort integration with raw HeyAPI errors.
  • bunx tsc --noEmit clean (pre-existing errors in local-mode.ts are on main).
  • bun run lint clean (no new warnings in changed files).
    Link to Devin session: https://app.devin.ai/sessions/c2e17ff1867f4ebd90aac007ea0f5453
    Requested by: @ashleeradka

…on calls

Closes LUM-2199

- Add bestEffort: true to captureError in use-conversation-history.ts
  (WEB-7: 77 events from 503 'still starting up' during pagination)
- Add bestEffort: true to captureError in web-search-card.tsx
  (WEB-2H: raw {detail} errors from HeyAPI throwOnError)
- Extend isExpectedDaemonTransientError() to detect raw {detail: string}
  objects thrown by HeyAPI's throwOnError — not just ApiError instances.
  HeyAPI throws the parsed JSON response body verbatim, so the function
  now matches known daemon transient detail substrings (still starting up,
  organization-id header, authentication credentials, bad gateway).

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment, CI, and merge conflict monitoring

@linear

linear Bot commented Jun 3, 2026

Copy link
Copy Markdown

LUM-2199

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 473c2d0777

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

if (cancelled) return;
setWebSearchHasStoredKey(false);
captureError(error, { context: "settings-ai-web-search-read-secret" });
captureError(error, { context: "settings-ai-web-search-read-secret", bestEffort: true });

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Gate or retry before suppressing secret-read errors

In platform mode this daemon call can fail with the org-header race that bestEffort now filters, but this effect is a one-shot read: it does not depend on useIsOrgReady() and has no retry/invalidation path when the org store finishes hydrating. In that scenario the catch above leaves webSearchHasStoredKey(false) and the settings card never re-reads the existing key, so suppressing the error here hides a state that can leave the UI incorrectly thinking no key is stored; please gate the request or retry before treating it as best-effort.

Useful? React with 👍 / 👎.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 3 additional findings.

Open in Devin Review

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant