PostHog track API backend #1284
Conversation
| Warning Rate limit exceeded@oscartbeaumont has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 15 minutes and 32 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughResolves and seeds a single Changes
Sequence Diagram(s)sequenceDiagram participant Init as App Init participant Store as GeneralSettingsStore participant PH as PostHog Module participant Event as Event Capture Init->>Init: Resolve server_url (debug env → stored → compile-time/default) alt resolved from env Init->>Store: Persist server_url end Init->>PH: set_server_url(resolved_url) PH->>PH: store cloud/self-hosted flag (OnceLock + RwLock) Init->>Init: Construct App with resolved server_url Note over Event,PH: Later — capturing an event Event->>PH: async_capture_event(raw_event) PH->>PH: read backend flag PH->>PH: enrich event (cap_version, cap_backend, os, arch) PH->>Event: posthog_rs::capture(enriched_event) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/desktop/src-tauri/src/lib.rs(3 hunks)apps/desktop/src-tauri/src/posthog.rs(4 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
**/*.rs: Format Rust code usingrustfmtand ensure all Rust code passes workspace-level clippy lints.
Rust modules should be named with snake_case, and crate directories should be in kebab-case.
Files:
apps/desktop/src-tauri/src/lib.rsapps/desktop/src-tauri/src/posthog.rs
🧬 Code graph analysis (2)
apps/desktop/src-tauri/src/lib.rs (2)
apps/desktop/src-tauri/src/posthog.rs (1)
set_server_url(70-75)apps/desktop/src-tauri/src/general_settings.rs (2)
update(222-231)get(208-219)
apps/desktop/src-tauri/src/posthog.rs (2)
apps/desktop/src-tauri/src/lib.rs (3)
std(1518-1536)std(1561-1583)set_server_url(1882-1888)apps/desktop/src-tauri/src/upload.rs (1)
spawn(335-373)
🔇 Additional comments (4)
apps/desktop/src-tauri/src/lib.rs (1)
1883-1886: LGTM! Proper state synchronization.The function correctly updates both the PostHog backend state and the in-memory
app.server_url, ensuring consistency across the application.apps/desktop/src-tauri/src/posthog.rs (3)
1-4: LGTM! Appropriate imports for thread-safe state management.The added synchronization primitives (
OnceLock,RwLock,PoisonError) are well-suited for managing the per-thread CAP Cloud flag.
27-55: LGTM! Clean separation of concerns.The refactoring to remove global properties from the
Fromimplementation and inject them inasync_capture_eventcentralizes property management and keeps the conversion focused on event-specific data.
82-108: LGTM! Property injection correctly implemented.The function properly enriches events with global properties before capture. The handling of
API_SERVER_IS_CAP_CLOUDbeingNone(returning"unknown") is appropriate for events captured beforeset_server_urlis called.Note: This ties into the initialization order issue flagged in
lib.rs- ensureset_server_urlis called beforeinitto avoid events being tagged withbackend="unknown".
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/desktop/src-tauri/src/posthog.rs(4 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
**/*.rs: Format Rust code usingrustfmtand ensure all Rust code passes workspace-level clippy lints.
Rust modules should be named with snake_case, and crate directories should be in kebab-case.
Files:
apps/desktop/src-tauri/src/posthog.rs
🧬 Code graph analysis (1)
apps/desktop/src-tauri/src/posthog.rs (2)
apps/desktop/src-tauri/src/lib.rs (3)
std(1518-1536)std(1561-1583)set_server_url(1882-1888)apps/desktop/src-tauri/src/upload.rs (1)
spawn(335-373)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Build Desktop (aarch64-apple-darwin, macos-latest)
- GitHub Check: Build Desktop (x86_64-pc-windows-msvc, windows-latest)
- GitHub Check: Analyze (rust)
🔇 Additional comments (4)
apps/desktop/src-tauri/src/posthog.rs (4)
1-4: LGTM!The imports are appropriate for the thread-safe state management pattern used in this module.
25-57: LGTM!The event conversion is clean and uses appropriate error handling for analytics properties. The error-logging pattern (
.map_err().ok()) ensures failures don't impact the main operation flow.
77-77: LGTM!The use of
OnceLock<RwLock<Option<bool>>>is the appropriate pattern for thread-safe, lazily-initialized global state. TheOption<bool>allows representing three states (cloud/self-hosted/unknown), which is correctly utilized inasync_capture_event.
79-114: LGTM!The event enrichment and capture logic is well-structured:
- Non-blocking async task spawn prevents impacting the main thread
- Property enrichment happens after event conversion, maintaining separation of concerns
- Thread-safe read from
API_SERVER_IS_CAP_CLOUDwith proper poison error handling- The three-state
cap_backendmapping (cloud/self_hosted/unknown) provides useful telemetry
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
apps/desktop/src-tauri/src/lib.rs (1)
2229-2253: Initialize PostHog after resolving server_url to avoid “unknown” cap_backend on early events.posthog::init() is called at Line 1944, before set_server_url runs here; events emitted in that window won’t be enriched correctly.
Move init to run after set_server_url (or call set_server_url earlier). Minimal change:
@@ - posthog::init(); + // defer until server_url is resolvedThen initialize here, right after set_server_url:
posthog::set_server_url(&server_url); + posthog::init();Also, tiny nit: the closure param in GeneralSettingsStore::update doesn’t need
mut. Optional cleanup:- GeneralSettingsStore::update(&app, |mut s| { + GeneralSettingsStore::update(&app, |s| { s.server_url = server_url.clone(); })
- Should release builds also honor a runtime VITE_SERVER_URL (std::env::var) or is compile‑time + store the intended contract? If runtime override is desired in release, mirror the debug path without cfg!(debug_assertions).
🧹 Nitpick comments (1)
apps/desktop/src-tauri/src/lib.rs (1)
1883-1886: Persist setting to GeneralSettingsStore to survive app restarts.Right now this only updates in‑memory state and PostHog flag. If the UI doesn’t also save to settings, the change is lost after restart. Persist here to make the command authoritative.
Apply this diff:
async fn set_server_url(app: MutableState<'_, App>, server_url: String) -> Result<(), ()> { - let mut app = app.write().await; - posthog::set_server_url(&server_url); - app.server_url = server_url; + let mut app = app.write().await; + posthog::set_server_url(&server_url); + app.server_url = server_url.clone(); + // Persist for future launches (non-fatal on error) + GeneralSettingsStore::update(&app.handle, |s| { + s.server_url = server_url; + }) + .map_err(|err| warn!("Error persisting server_url: {err}")) + .ok(); Ok(()) }Confirm the UI also writes GeneralSettingsStore.server_url. If so, we can keep this as a safety net; otherwise this becomes the primary persistence path.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/desktop/src-tauri/src/lib.rs(3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
**/*.rs: Format Rust code usingrustfmtand ensure all Rust code passes workspace-level clippy lints.
Rust modules should be named with snake_case, and crate directories should be in kebab-case.
Files:
apps/desktop/src-tauri/src/lib.rs
🧬 Code graph analysis (1)
apps/desktop/src-tauri/src/lib.rs (2)
apps/desktop/src-tauri/src/posthog.rs (1)
set_server_url(70-75)apps/desktop/src-tauri/src/general_settings.rs (2)
get(208-219)update(222-231)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Build Desktop (aarch64-apple-darwin, macos-latest)
- GitHub Check: Build Desktop (x86_64-pc-windows-msvc, windows-latest)
- GitHub Check: Analyze (rust)
This will help us tell if all the uploader bugs are fixed because we think most of the current errors are people hosting old versions of Cap web.
Summary by CodeRabbit
Bug Fixes
Chores