Skip to content

fix(sentry): tag Tauri shell events with cached user uid#2320

Merged
senamakel merged 2 commits into
tinyhumansai:mainfrom
senamakel:fix/sentry-uid-tauri-shell
May 20, 2026
Merged

fix(sentry): tag Tauri shell events with cached user uid#2320
senamakel merged 2 commits into
tinyhumansai:mainfrom
senamakel:fix/sentry-uid-tauri-shell

Conversation

@senamakel
Copy link
Copy Markdown
Member

@senamakel senamakel commented May 20, 2026

Summary

  • The Tauri shell's Sentry before_send was explicitly setting event.user = None, so desktop events landed without a uid attached and Sentry could not count unique users impacted by an issue.
  • Since fix(core,cef): run core in-process and stop orphaning CEF helpers on Cmd+Q #1061 the sidecar was removed and the core runs in-process inside the shell — this filter is what runs for ~all production desktop events. The standalone openhuman-core binary in src/main.rs already attaches the uid via peek_cached_current_user_identity; the shell now mirrors that.
  • Only id is carried (no email, name, or IP), consistent with send_default_pii: false.

Test plan

  • cargo check --manifest-path app/src-tauri/Cargo.toml ✅ (verified locally)
  • Manual: trigger a Sentry event from the desktop app after login (e.g. OPENHUMAN_TAURI_SENTRY_TEST=message) and confirm the event in Sentry shows the user id under "User" and contributes to "users impacted"
  • Manual: trigger a Sentry event before login and confirm user is absent (cache not yet populated)

Summary by CodeRabbit

  • Improvements

    • Enhanced error tracking to attribute crash reports to individual users using a privacy-preserving identifier.
    • Increased startup readiness timeout to reduce intermittent startup flakiness.
  • Tests

    • Improved test resilience by handling locked resources more gracefully to prevent cascading test failures.

Review Change Stack

Since the sidecar was removed (tinyhumansai#1061) and the core runs in-process
inside the Tauri host, the shell's `before_send` is the filter that
fires for ~all desktop Sentry events. It was explicitly setting
`event.user = None`, so Sentry could not count unique users impacted
by an issue.

Mirror the standalone `openhuman-core` binary (`src/main.rs`) and
attach the cached account uid via `peek_cached_current_user_identity`.
Only `id` is carried — no email, name, or IP — keeping the privacy
contract aligned with `send_default_pii: false`.
@senamakel senamakel requested a review from a team May 20, 2026 09:12
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 20, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 79eea952-f98b-487e-9f15-d072440647ef

📥 Commits

Reviewing files that changed from the base of the PR and between 4d35de1 and 993ab7e.

📒 Files selected for processing (2)
  • app/src-tauri/src/core_process.rs
  • app/src-tauri/src/core_process_tests.rs

📝 Walkthrough

Walkthrough

Sentry before_send now attaches the cached current user's ID to events; the embedded core readiness poll retries up to 100 times (10s); test env_lock() recovers from poisoned Mutexes instead of panicking.

Changes

Tauri shell adjustments

Layer / File(s) Summary
Sentry event user identity population
app/src-tauri/src/lib.rs
The before_send handler calls peek_cached_current_user_identity() and sets event.user to sentry::User { id: Some(id), ..Default::default() }, replacing the previous behavior that left event.user unset.
Embedded core readiness polling window
app/src-tauri/src/core_process.rs
The readiness polling loop now iterates 100 times with 100ms sleeps (10s budget), updated from 40 iterations; comments updated to document CI flakiness history and new timeout.
Test env_lock poison recovery
app/src-tauri/src/core_process_tests.rs
env_lock() now uses `unwrap_or_else(

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • tinyhumansai/openhuman#1719: Also modifies the Tauri shell’s Sentry before_send handler (adds is_session_expired_event suppression), related to observability and event handling.

Suggested labels

working

Poem

🐰 I nibble at logs in the soft moonlight,
I tag each hop with an ID—hidden, light.
Polls wait longer while cores find their pace,
Tests mend their fences after a race.
A tiny rabbit cheers this tidy, quiet night.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly describes the main change: tagging Tauri shell Sentry events with cached user uid. This aligns with the primary modification in lib.rs where event.user is now populated with the cached user's id.
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.

Warning

Review ran into problems

🔥 Problems

Git: Failed to clone repository. Please run the @coderabbitai full review command to re-trigger a full review. If the issue persists, set path_filters to include or exclude specific files.


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

coderabbitai[bot]
coderabbitai Bot previously approved these changes May 20, 2026
Two pre-existing CI flakes in core_process tests:

1. `ensure_running` had a hardcoded 4s readiness budget (40 × 100ms).
   The embedded core's JSON-RPC controller registry has grown enough
   that cold-start under CI worker load occasionally exceeded 4s,
   producing "core process did not become ready" failures. Bumped to
   10s (100 × 100ms) — still well under any user-visible startup
   expectation, matches the upper end of observed cold-start times.

2. When `ensure_running_falls_back_for_unknown_listener_on_port`
   panicked while holding `env_lock()`, the next two tests
   (`..7789_when_7788_is_busy` and `..reuses_unknown_listener..`)
   cascade-failed with `expect("env lock poisoned")`, turning one
   real flake into three. The lock only serializes env-var mutation —
   the inner `()` has no state poisoning could corrupt — so recover
   via `into_inner()`.
@coderabbitai coderabbitai Bot added the working A PR that is being worked on by the team. label May 20, 2026
@M3gA-Mind
Copy link
Copy Markdown
Contributor

@senamakel CI is failing on changes in this PR — please fix before review.

1 similar comment
@M3gA-Mind
Copy link
Copy Markdown
Contributor

@senamakel CI is failing on changes in this PR — please fix before review.

@senamakel senamakel merged commit 3dbedd2 into tinyhumansai:main May 20, 2026
25 of 31 checks passed
mtkik pushed a commit to mtkik/openhuman-meet that referenced this pull request May 21, 2026
CodeGhost21 pushed a commit to CodeGhost21/openhuman that referenced this pull request May 22, 2026
AusAgentSmith pushed a commit to AusAgentSmith/openhuman that referenced this pull request May 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

working A PR that is being worked on by the team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants