Skip to content

fix(conversations): catch loadThreads rejection on mount to silence Sentry noise - #1513

Merged
senamakel merged 1 commit into
tinyhumansai:mainfrom
senamakel:fix/conversations-unhandled-threads-list-rejection
May 12, 2026
Merged

senamakel merged 1 commit into
tinyhumansai:mainfrom
senamakel:fix/conversations-unhandled-threads-list-rejection

Conversation

@senamakel

@senamakel senamakel commented May 12, 2026 •

Copy link
Copy Markdown
Member

Summary

  • Adds a .catch() to the mount-time dispatch(loadThreads()).unwrap().then(...) chain in Conversations.tsx so a failed initial thread fetch no longer surfaces as an unhandled promise rejection.

Problem

Sentry issue OPENHUMAN-REACT-X (UnhandledRejection: Non-Error promise rejection captured with value: Core RPC openhuman.threads_list timed out after 30000ms).

When the core RPC openhuman.threads_list timed out on app mount, the chain at app/src/pages/Conversations.tsx:257 had no .catch(). createAsyncThunk's .unwrap() re-throws Redux's SerializedError (a plain object, not an Error instance), so the rejection became unhandled and Sentry tagged it as a "Non-Error promise rejection."

Solution

  • Attach a .catch() after the existing .then() that bails when the effect has been cancelled and otherwise logs the message via console.warn.
  • No behavioral change: the slice's loadThreads.rejected case already drives UI state — this just suppresses the unhandled rejection at the promise boundary.

Submission Checklist

  • N/A: pure rejection-handling fix on a one-shot mount effect with no new branches; the timeout path is already covered by the slice's rejected reducer
  • N/A: behavioural change is rejection-handling only; no new code paths exercised
  • N/A: behaviour-only change, no feature rows affected
  • N/A: no feature matrix rows touched
  • N/A: no new network calls introduced
  • N/A: no release-cut surfaces touched
  • N/A: no linked issue (Sentry-driven fix); Sentry issue id is OPENHUMAN-REACT-X

Impact

  • Desktop UI: a threads_list RPC timeout on app mount no longer produces an unhandled rejection or a Sentry event for the user; the failure is logged to the console and the UI continues with whatever empty/cached state the slice reducer left behind.
  • No performance, security, or migration impact.

Related

  • Closes: Sentry OPENHUMAN-REACT-X
  • Follow-up PR(s)/TODOs:

AI Authored PR Metadata (required for Codex/Linear PRs)

Linear Issue

  • Key: N/A
  • URL: N/A

Commit & Branch

  • Branch: fix/conversations-unhandled-threads-list-rejection
  • Commit SHA: aa04aeb

Validation Run

  • N/A: format unchanged (single .catch() block added, Prettier-conformant)
  • pnpm typecheck (run as npx tsc --noEmit — clean)
  • N/A: no Vitest covers this mount-time IIFE branch today; behaviour-equivalent change
  • N/A: no Rust changes
  • N/A: no Tauri changes

Validation Blocked

  • command: N/A
  • error: N/A
  • impact: N/A

Behavior Changes

  • Intended behavior change: a rejected loadThreads on mount is caught and logged instead of surfacing as an unhandled promise rejection.
  • User-visible effect: no Sentry event / no browser-console "Uncaught (in promise)" when the initial openhuman.threads_list RPC fails or times out.

Parity Contract

  • Legacy behavior preserved: success path of the .then() chain is untouched; the rejected-action UI state from the slice still flows through reducers.
  • Guard/fallback/dispatch parity checks: the new .catch() short-circuits when the effect's cancelled flag is set, matching the existing pattern in the .then() callback.

Duplicate / Superseded PR Handling

  • Duplicate PR(s): N/A
  • Canonical PR: N/A
  • Resolution: N/A

Summary by CodeRabbit

  • Bug Fixes
    • Enhanced error handling when loading conversations with improved logging for better visibility into issues.
    • Improved lifecycle management for the conversations view to prevent issues during navigation.

Review Change Stack

…entry noise

The mount-time `dispatch(loadThreads()).unwrap().then(...)` chain had no
`.catch()`, so when the core RPC `openhuman.threads_list` timed out,
`.unwrap()` re-threw Redux's `SerializedError` (a plain object, not an
`Error`). That surfaced as Sentry's "Non-Error promise rejection captured
with value: Core RPC openhuman.threads_list timed out after 30000ms"
(OPENHUMAN-REACT-X).

The slice's `loadThreads.rejected` case already handles UI state — just
swallow the rejection at the promise boundary so it isn't unhandled.
@senamakel
senamakel requested a review from a team May 12, 2026 02:46
@coderabbitai

coderabbitai Bot commented May 12, 2026 •

Copy link
Copy Markdown
Contributor
📝 Walkthrough

Walkthrough

The Conversations component's mount-time loadThreads() dispatch now includes a .catch handler that logs a warning with a normalized error message when thread loading fails, while respecting the component's cancelled flag to prevent side effects after unmount.

Changes

Mount-time thread loading error handling

Layer / File(s) Summary
Mount-time loadThreads error handler
app/src/pages/Conversations.tsx
Promise rejection handler on the mount-time loadThreads() call logs a warning with the error message when thread loading fails, guarded by the cancelled flag to avoid side effects after unmount.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • tinyhumansai/openhuman#1192: Both PRs add error-handling or guards to loadThreads() call sites in different components (Conversations mount vs CoreStateProvider refresh).

Poem

A catch block blooms in Conversations' spring,
Error logs whisper when threads won't sing,
Unmount flags stand guard with gentle grace,
Warnings flow softly through this humble space. 🐰

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title clearly and concisely summarizes the main change: adding error handling to silence Sentry noise from an unhandled promise rejection.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
app/src/pages/Conversations.tsx (1)

257-296: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win

Refactor this effect to use async/await and namespaced debug logging

Lines 257–296 use promise chaining instead of async/await, and console.warn instead of namespaced debug logging, both violating the coding guidelines. Convert to an async IIFE with try/catch and replace the console.warn with a namespaced debug logger for frontend logging compliance.

Proposed refactor
   useEffect(() => {
     let cancelled = false;
 
-    void dispatch(loadThreads())
-      .unwrap()
-      .then(data => {
+    void (async () => {
+      try {
+        const data = await dispatch(loadThreads()).unwrap();
         // [`#1123`] Commented out — welcome-agent onboarding replaced by Joyride walkthrough
         // if (cancelled || skipInitialThreadSelectionRef.current) return;
         if (cancelled) return;
@@
         if (data.threads.length > 0) {
@@
         } else {
           void handleCreateNewThread();
         }
-      })
-      .catch(err => {
+      } catch (err) {
         if (cancelled) return;
         const message = err instanceof Error ? err.message : String(err?.message ?? err);
-        console.warn('[conversations] loadThreads failed on mount:', message);
+        debug('[conversations] loadThreads failed on mount:', message);
-      });
+      }
+    })();
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app/src/pages/Conversations.tsx` around lines 257 - 296, Replace the promise
chain starting at dispatch(loadThreads()).unwrap() with an async IIFE:
immediately await dispatch(loadThreads()).unwrap() inside a try block, keep the
cancelled guard checks before and after the await, and preserve the
thread-selection logic that uses store.getState().thread, setSelectedThread and
loadThreadMessages; in the catch block use the project's namespaced debug/logger
(e.g., logger.warn or debug('conversations')) instead of console.warn and log
the error message (err instanceof Error ? err.message : String(err)); ensure
handleCreateNewThread() is awaited/called the same way as before when no threads
are returned.
🧹 Nitpick comments (1)
app/src/pages/Conversations.tsx (1)

292-295: ⚡ Quick win

Replace direct console.warn with namespaced debug logging

Line 295 introduces direct console logging; this conflicts with the frontend logging guideline. Please use the project’s namespaced debug pattern and keep detail dev-scoped.

As per coding guidelines, “Use namespaced debug for frontend logging with dev-only detail; avoid logging secrets or full PII.”

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app/src/pages/Conversations.tsx` around lines 292 - 295, The catch block
inside loadThreads in Conversations.tsx currently uses console.warn; replace it
with the project's namespaced debug logger (e.g., debug('conversations') or a
module-level debugConversations instance) and call that instead, keeping the
same message string but ensuring no sensitive/PII data is logged; add or reuse
the appropriate import/initializer for the namespaced debug logger at the top of
the file and swap the console.warn call in the catch (which references cancelled
and message) to debugConversations(...) so logs remain dev-scoped.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@app/src/pages/Conversations.tsx`:
- Around line 257-296: Replace the promise chain starting at
dispatch(loadThreads()).unwrap() with an async IIFE: immediately await
dispatch(loadThreads()).unwrap() inside a try block, keep the cancelled guard
checks before and after the await, and preserve the thread-selection logic that
uses store.getState().thread, setSelectedThread and loadThreadMessages; in the
catch block use the project's namespaced debug/logger (e.g., logger.warn or
debug('conversations')) instead of console.warn and log the error message (err
instanceof Error ? err.message : String(err)); ensure handleCreateNewThread() is
awaited/called the same way as before when no threads are returned.

---

Nitpick comments:
In `@app/src/pages/Conversations.tsx`:
- Around line 292-295: The catch block inside loadThreads in Conversations.tsx
currently uses console.warn; replace it with the project's namespaced debug
logger (e.g., debug('conversations') or a module-level debugConversations
instance) and call that instead, keeping the same message string but ensuring no
sensitive/PII data is logged; add or reuse the appropriate import/initializer
for the namespaced debug logger at the top of the file and swap the console.warn
call in the catch (which references cancelled and message) to
debugConversations(...) so logs remain dev-scoped.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: dc6b889d-5a84-4c90-8c34-3dfb41d55390

📥 Commits

Reviewing files that changed from the base of the PR and between 6729fe9 and aa04aeb.

📒 Files selected for processing (1)
  • app/src/pages/Conversations.tsx

@senamakel
senamakel merged commit ba3e116 into tinyhumansai:main May 12, 2026
17 of 20 checks passed
senamakel added a commit to senamakel/openhuman that referenced this pull request May 12, 2026
…anches

The mount-time .catch() block added in tinyhumansai#1513 normalises Redux's
SerializedError plain object into a string for console.warn. Pull that
logic into an exported pure helper so it can be unit-tested without
mounting the full page, then exercise every branch (Error,
SerializedError-shaped object, plain object, string, null/undefined,
non-string message field). Coverage Gate on changed lines goes from
0% (the original 4-line inline catch was untested) to ≥80%.

No behavior change beyond the refactor.
@senamakel
senamakel deleted the fix/conversations-unhandled-threads-list-rejection branch May 12, 2026 03:07
AusAgentSmith pushed a commit to AusAgentSmith/openhuman that referenced this pull request May 23, 2026
senamakel added a commit to nocstah/openhuman that referenced this pull request Sep 11, 2026
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