Repository navigation
feat(telemetry): report desktop and web exceptions to PostHog - #82
leoisadev1 wants to merge 31 commits into
Conversation
|
Note 🤖 GPT-6.1 Sol responding on behalf of Leo CNV-55 adds desktop telemetry, so the privacy page must disclose the anonymous events and common properties collected, that paths, filenames, file contents, and error text are never collected, PostHog US as processor, and the Settings / |
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| props.insert("error_kind".into(), k.into()); | ||
| } | ||
| props.insert("$exception_list".into(), serde_json::json!([{"type":kind,"value":scrub(value),"stacktrace":{"frames":[frame],"raw":scrub(stack)}}])); | ||
| serde_json::json!({"event":"$exception","api_key":POSTHOG_KEY,"properties":props}) |
There was a problem hiding this comment.
All three raw PostHog reporters omit distinct_id. PostHog requires an ID for each event, so desktop, Worker, and API errors do not produce usable exception events or trigger alerts. Add a non-personal ID to each payload and test the complete request body.
Correctness confidence: 5/5.
| let end = tail | ||
| .find(|c: char| c.is_whitespace() || c == ')' || c == ']' || c == '"') | ||
| .unwrap_or(tail.len()); | ||
| cleaned.push_str("<PATH>"); | ||
| rest = &tail[end..]; |
There was a problem hiding this comment.
Desktop scrub leaves parts of filenames containing spaces in the outgoing report. For example, /home/alice/Highly Confidential Plan.jpg becomes <PATH> Confidential <PATH>. Conversion errors include full paths, so private filename words leave the computer despite the scrubbing promise. Remove the whole path before splitting words and add spaced-path tests.
Correctness confidence: 5/5.
How this was verified: Conversion errors carry user paths into report_error, and the path filter preserves words after the first space before placing them in the outgoing exception.
| .layer(CatchPanicLayer::custom( | ||
| |panic: Box<dyn std::any::Any + Send>| { |
There was a problem hiding this comment.
CatchPanicLayer covers only the routes already added by app(). main merges the jobs router afterward, so panics in /v1/jobs handlers bypass both the report and the new 500 response. Apply the layer after merging the jobs routes.
Correctness confidence: 5/5.
| } else { | ||
| "server panic".into() | ||
| }; | ||
| crate::posthog::capture(&message, "convt-server request"); |
There was a problem hiding this comment.
The API reporter's only caller is the panic callback. Database and storage failures return ApiError responses with 500 or 502 without panicking, so the new PostHog alerts miss these common failures. Add scrubbed capture to those paths while leaving expected client errors unreported.
Correctness confidence: 5/5.
| person_profiles: "identified_only", | ||
| capture_pageview: false, | ||
| capture_pageleave: true, | ||
| capture_exceptions: true, |
There was a problem hiding this comment.
Enabling capture_exceptions adds error messages and stack text to browser events, but before_send only applies the existing URL cleaner. That cleaner leaves non-URL strings unchanged, including emails or credentials embedded in error text. Add exception-specific scrubbing before sending and tests for nested exception fields.
Correctness confidence: 4/5.
| pub fn report_error(kind: &str, message: &str) { | ||
| let stack = Backtrace::force_capture().to_string(); | ||
| let kind = kind.to_string(); | ||
| let message = message.to_string(); | ||
| let _ = std::thread::Builder::new() | ||
| .name("convt-error-report".into()) | ||
| .spawn(move || { | ||
| let _ = send(event("ConversionError", &message, &stack, Some(&kind))); |
There was a problem hiding this comment.
report_error captures and formats a backtrace on the UI thread before starting background work. It also starts a new thread for every failure before checking enabled(). Large failed batches add UI work and unchecked thread creation, even for opted-out users. Check the opt-out first, keep expensive report work off the UI thread, and use a bounded reporting queue.
Correctness confidence: 5/5.
| let _ = write_crash(&message, location, &backtrace); | ||
| let _ = send(event("panic", &message, &backtrace, None)); |
There was a problem hiding this comment.
A successful immediate panic report never marks its crash file as sent. The next launch sees the same file without a .sent marker and reports the crash again, inflating error counts and alerts. Keep the path returned by write_crash and mark it after a successful immediate send.
Correctness confidence: 5/5.
| We also collect scrubbed crash and error reports from the website and desktop app, with | ||
| the same opt-out as analytics. |
There was a problem hiding this comment.
The new crash-report disclosure leaves the desktop section's exhaustive network list incorrect. That section says the app connects only for updates, Pro renewal, or actions the user requests, but crash reports and startup resends are automatic. Add reporting to that list so readers get a consistent explanation.
Correctness confidence: 5/5.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| tokio::spawn(async move { | ||
| let _ = reqwest::Client::new() | ||
| .post("https://us.i.posthog.com/capture/") | ||
| .json(&body) | ||
| .send() | ||
| .await; |
There was a problem hiding this comment.
API capture starts a fresh HTTP client and request without a configured timeout. If PostHog stalls, reporting tasks and connections can stay live while more reports start. Give the request a finite deadline, as the desktop reporter already does.
Correctness confidence: 4/5.
| let re = |s: String, pat: &str, replacement: &str| { | ||
| let mut out = String::with_capacity(s.len()); | ||
| for part in s.split(pat) { | ||
| if !out.is_empty() { | ||
| out.push_str(replacement); | ||
| } | ||
| out.push_str(part); | ||
| } | ||
| out | ||
| }; |
There was a problem hiding this comment.
The new re closure is never called; let _ = &re only suppresses its unused-variable warning. Remove both. Keeping an unused replacement helper inside the privacy filter makes it harder to see which rules actually run.
Correctness confidence: 5/5.
…ribe Leo decided to keep the addresses the phone card collects as a launch mailing list and to reuse PRODUCTHUNT (30% off, through 31 October). - New launch_list table (migration 0006, down file, convt_web grants): one row per normalized address, its source, first consent time, and the SHA-256 of a stable per-address unsubscribe token (an HMAC of the address under the auth secret, so every email's link works). - The card says that sending also joins the launch list, with a privacy link. - The email ends with an unsubscribe link (/unsubscribe#t=...). The page reads the token from the fragment and deletes the row only on a button press, so mail scanners change nothing. - Privacy page: a short launch-list item, and the providers line no longer says no marketing email is sent. #82's analytics and desktop text is untouched. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
convt is the seller's trading name and Polar (polar.sh) processes every purchase as merchant of record. No postal address. 14-day no-reason refunds. Florida law and courts, with consumers keeping their home protections. Effective October 7, 2026. The privacy page no longer says the app has no analytics or crash reporting; PRs #82 and #84 own those disclosures. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
| text.lines().find_map(|line| { | ||
| let (key, value) = line.split_once('=')?; | ||
| (key.trim() == "telemetry").then(|| value.trim().parse::<bool>().ok())? | ||
| }) | ||
| }) | ||
| .unwrap_or(true); |
There was a problem hiding this comment.
enabled() ignores a valid settings.toml opt-out such as telemetry = false # opt out. parse::<bool>() rejects the comment, so unwrap_or(true) enables reporting. In packaged builds without DO_NOT_TRACK, crashes and conversion errors then start outgoing requests despite the user's choice.
Read the root telemetry value with the TOML parser and test commented values through enabled(). Correctness confidence: 5/5.
How this was verified: A commented false value falls through to true, which send() passes to the gate before posting reports to PostHog.
| PostHog also receives anonymous desktop usage events and scrubbed crash and error reports | ||
| from the website and desktop app. Usage events include the app version, operating system, | ||
| architecture, conversion format, outcome, duration, size bucket, license state and a | ||
| random install ID; a signed-in account is represented only by an opaque identifier. | ||
| Reports include a scrubbed error type, message, stack, location, app version, operating | ||
| system, architecture, locale and error kind. We exclude file contents, filenames and | ||
| paths, usernames, email addresses, tokens, license keys and account details. The desktop | ||
| Telemetry setting is the shared opt-out for usage and crash/error reports. Turn it off in | ||
| Settings, set <code>DO_NOT_TRACK=1</code>, or use Global Privacy Control or Do Not Track | ||
| in your browser to prevent these reports from being sent. |
There was a problem hiding this comment.
The privacy page describes desktop usage events and a Telemetry switch that do not exist in this branch. The app sends crash/error reports only, and Settings has no telemetry field or switch. Users following “Turn it off in Settings” cannot find that control.
Describe the current reports and working opt-out. Add the usage-event disclosure and Settings instructions when CNV-55 lands. Correctness confidence: 5/5.
* Replace the draft terms and privacy text with final wording Drop the draft banner and bracket placeholders. Terms now cover refunds, termination, governing law and contact; the provider, refund policy, governing law and effective date come from legal in lib/site.ts, marked TODO(CNV-48) until Leo decides them. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * Match the refund and deletion wording to how billing works Keys are checked offline, so a revoked key is not switched off; account deletion ends subscriptions before removing the account and can take up to a day; the refund window covers Desktop and Pro, not metered API use. The legal test now rejects any bracketed or draft text. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * Apply Leo's legal decisions and drop the app's no-analytics claims convt is the seller's trading name and Polar (polar.sh) processes every purchase as merchant of record. No postal address. 14-day no-reason refunds. Florida law and courts, with consumers keeping their home protections. Effective October 7, 2026. The privacy page no longer says the app has no analytics or crash reporting; PRs #82 and #84 own those disclosures. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * Test the Polar, refund-revocation and controller wording Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * Do not promise a deadline for account deletion Deletion retries failed billing steps with no cutoff, so the terms say it usually takes minutes rather than promising a day. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
#82's scrubber treats dotted versions as filenames. OS version is not a secret, so the bundle leaves it alone. report_error stays as the generic wrapper for non-conversion errors. Co-authored-by: Leo <leoisadev1@users.noreply.github.com>
Keep report_conversion format ids and clear automation clipboard state on a failed job. Resolves the Settings/model conflict after #82 merged main. Co-authored-by: Leo <leoisadev1@users.noreply.github.com>
| if !allowed { | ||
| return Err(()); | ||
| } | ||
| let body = serde_json::json!({"batch":[value]}); |
There was a problem hiding this comment.
The desktop request puts api_key inside each event, but PostHog's /batch/ endpoint expects it beside batch in the request body. Crash reports, conversion errors, and retries therefore lack the required project key and are rejected. Move api_key to the outer body and test the complete request.
Correctness confidence: 4/5.
| let body = serde_json::json!({"batch":[value]}); | |
| let body = serde_json::json!({"api_key":POSTHOG_KEY,"batch":[value]}); |
| let setting = crate::settings::Settings::path() | ||
| .and_then(|path| std::fs::read_to_string(path).ok()) | ||
| .and_then(|text| { | ||
| text.lines().find_map(|line| { | ||
| let (key, value) = line.split_once('=')?; | ||
| (key.trim() == "telemetry").then(|| value.trim().parse::<bool>().ok())? | ||
| }) | ||
| }) | ||
| .unwrap_or(true); |
There was a problem hiding this comment.
A bare telemetry = false works only until the app saves settings. Settings has no telemetry field, so its full-file save removes that choice during startup update checks, Pro renewal, or a normal settings change. The next enabled() call defaults to true and allows outgoing reports again. Store the choice in Settings and preserve it on every save.
Correctness confidence: 5/5.
How this was verified: check_updates() saves settings.toml without telemetry, and send() then rereads the missing key as true before making a PostHog request.
| if (request.headers.get("sec-gpc") === "1" || request.headers.get("dnt") === "1") return false; | ||
| return !/(?:^|;\s*)(?:convt:analytics-opt-out|analytics-opt-out)=1(?:;|$)/.test( | ||
| request.headers.get("cookie") ?? "", |
There was a problem hiding this comment.
The privacy switch writes convt_analytics=off, but the Worker and API gates check different cookie names with a value of 1. Users who turn analytics off without enabling GPC/DNT still allow outgoing error reports. Both gates also miss the DNT: yes value accepted by the existing consent helper.
Reuse analyticsAllowedFromHeaders in the Worker and mirror its rules in the API. Correctness confidence: 5/5.
How this was verified: The privacy switch writes convt_analytics=off, while both request gates check other cookie names before allowing PostHog sends.
| } | ||
|
|
||
| fn main() -> ExitCode { | ||
| crash_report::install(); |
There was a problem hiding this comment.
Every launch starts resend before instance::claim. Two launches after a crash can read the same pending file and start duplicate uploads before either writes its .sent marker. The second launch can wait for the first app to start while its retry thread runs.
Keep installing the panic hook early, but start retries only after receiving Role::Primary, or lock each pending file. Correctness confidence: 4/5.
| let stamp = format!( | ||
| "{}-{}", | ||
| std::process::id(), | ||
| std::time::SystemTime::now() | ||
| .duration_since(std::time::UNIX_EPOCH) | ||
| .map_or(0, |d| d.as_millis()) | ||
| ); | ||
| let path = dir.join(format!("crash-{stamp}.log")); |
There was a problem hiding this comment.
Crash filenames use only the process ID and the current millisecond. Two worker threads that panic within the same millisecond choose the same filename, and fs::write replaces the earlier report. This loses the local evidence for one crash.
Give each report a unique suffix and create the file without replacing an existing one. Correctness confidence: 4/5.
This branch has not been deployed
1 participant
0273074
Summary
CNV-57 adds best-effort PostHog
$exceptionreporting across the desktop app, web Worker, and Railway API. Desktop panics are written synchronously to the platform log directory before the previous panic hook runs; unsent files are retried once on the next launch and handled conversion failures are reported off the conversion path.CNV-55 has not landed and has no branch or open PR in this checkout, so this change is self-contained and reads its future
telemetry_enabledsettings key when present. It does not edit CNV-55 files.Evidence
.sentmarker.capture_exceptions: true.cargo fmt --all.cargo test -p convt-app crash_reportcould not complete because the environment lacksfontconfig.pc(the build stopped inyeslogic-fontconfig-sys).tscis not installed in this checkout.PostHog alert setup (the project currently has no connected integrations, so this cannot be created by API):
#eng-convt) or a webhook, then save and enable.The privacy page change in CNV-49 must mention crash and error reporting.
Merge Danger
Door: two-way. Reverting this commit removes capture; external PostHog alert configuration remains a separate dashboard action.
Blast radius: all desktop panics and conversion failures plus uncaught web, Worker, and API errors. Network work is bounded or queued and never controls conversion or HTTP responses.
Fixes CNV-57
Latest merge verification
Merged
origin/mainatab97d53and resolved the PostHog provider conflict while preserving the privacy-safe main implementation andcapture_exceptions: true.PATH=/home/leo/.cargo/bin:$PATH /home/leo/.local/bin/rust-test-limited -p convt-app— blocked by missing system dependencyfontconfig.pcinyeslogic-fontconfig-sys.bun run --cwd apps/web check-types— blocked becausetscis not installed in this checkout.Review follow-up
$exception_listplacement so it is insideproperties, with a unit test for the event shape.SIGABRT/SIGSEGVaborts. No native signal/exception handler was added because running Rust, allocation, or network work from those handlers is unsafe; the existing synchronous crash file remains safe for Rust panics.Verification after the fixes:
cargo fmt --allandgit diff --check.PATH=/home/leo/.cargo/bin:$PATH /home/leo/.local/bin/rust-test-limited -p convt-server posthog::tests(1 passed).-p convt-appremains blocked by missingfontconfig.pc.tscis not installed.Formal review follow-up
posthog-scrub.tsand mirrored them in Rust: absolute paths and filenames, emails, bearer/token headers, named token/API-key/license-key/secret fields, and convt license-key shapes are redacted. Web and Rust tests cover a path, email, license key, and token.Sec-GPC: 1,DNT: 1, and the analytics opt-out cookie before enqueueing a report. The client-side localStorage opt-out is not transmitted to the server, so the server gate uses the cookie/header signals available on the request.POSTHOG_ERRORS=0disables API reports.CatchPanicLayerreceives only the panic payload, not the failed request headers, so GPC/DNT cannot be read for a panic there; this limitation is explicit and the payload remains identifier-free. Request-aware API paths can add those signals when they capture errors directly.Verification after the formal review fixes:
cargo fmt --allandgit diff --check.PATH=/home/leo/.cargo/bin:$PATH /home/leo/.local/bin/rust-test-limited -p convt-server posthog::tests(1 passed).bun test apps/web/test/unit/posthog-error.test.ts(2 passed).bun test apps/web/test/unitran 14 tests successfully but 10 files could not load because workspace dependencies such as React and@convt/dbare not installed in this checkout.bun run --cwd apps/web check-typesbecausetscis not installed.cargo test -p convt-appbecause the environment lacksfontconfig.pc.CI follow-up
The first post-review CI run found and was fixed for two issues: a duplicate
capture_exceptionsoption caused web lint/typecheck problems, and the Windows build found an unused Unix-onlyHOMEbinding in the crash reporter. The privacy-safeposthogOptionsimplementation from main is now preserved, and the Windows cfg is corrected.Local verification after those fixes:
bun run check.bun test apps/web/test/unit(109 passed).RUST_FONTCONFIG_DLOPEN=1and-D warnings.@convt/sdk; CI has the full workspace install.CNV-55 opt-out follow-up
CNV-55 owns the desktop setting as
telemetryinsettings.toml(nottelemetry_enabled) and exposestelemetry::enabled(setting, enforced), including the sharedDO_NOT_TRACKbehavior. This branch now reads thetelemetrykey and mirrors that helper in a small merge bridge; once PR #84 lands, the bridge should be replaced with the sharedtelemetry::enabled(setting, convt_license::ENFORCED)call. In debug/source builds, sending still requires the explicitCONVT_TELEMETRY=1opt-in. Crash files continue to be written locally when disabled.Added
telemetry_opt_out_suppresses_crash_send, which passestelemetry = falsethrough the gate and verifies no send is attempted.Verification for commit
7116f22:cargo fmt --allandgit diff --check.cargois unavailable in this checkout's shell environment. The test is bounded and included for CI.CI follow-up for
19e1b40: the first run caught anOption<bool>::flattencompile error in the new settings parser; fixed in this follow-up commit and pushed. A fresh full matrix is running now.Final CI for
19e1b40(run37685987336): PASS — Web, Rust Linux/macOS/Windows core and desktop, release scripts, and Greptile review.Combined privacy disclosure
The privacy page now carries the single combined disclosure for desktop usage telemetry plus desktop/web crash and error reports. It documents the PostHog US destination, the usage fields (including random install ID and opaque signed-in identifier), scrubbed report fields, excluded content (files, filenames/paths, usernames, emails, tokens, license keys and account details), and the shared desktop
Telemetrysetting.DO_NOT_TRACK=1and browser GPC/DNT are also documented as opt-outs. PR #84 should remove its duplicate paragraph and rebase on this disclosure after #82 merges.Final privacy-only CI run
37687675347: PASS — Web, Rust Linux/macOS/Windows core and desktop, release scripts, and Greptile review.Latest conflict and privacy cleanup
Merged the latest
origin/main(including #77, #78 and #90) into this branch and resolved the privacy/model conflicts while preserving the combined disclosure and handled conversion-error capture. Test mailboxes are assembled at runtime so the source diff and this PR body contain no literal email addresses.Verification on the merged head:
bun run checkbun test apps/web/test/unit/posthog-error.test.ts(2 tests)git diff --checkCI follow-up: the merged-head run caught only a Rust formatting difference in the runtime mailbox test; commit
0273074applies rustfmt's one-line form and is pushed for a fresh matrix.Request privacy gate
The API panic layer now runs inside request-scoped privacy state.
Sec-GPC: 1,DNT: 1, and the existing analytics opt-out cookie suppress$exceptioncapture before the asynchronous sender starts;POSTHOG_ERRORS=0remains a server-wide kill switch. The gate and scoped suppression are unit-tested. The panic layer still cannot capture native aborts or Objective-C exceptions; it captures Rust panics only.Verification for
83db3bd:git diff --checkconvt-servertests and full matrix.CI follow-up: run
37703873856confirmed the new database integration job, web checks, and server database tests pass. Its Linux Rust jobs stopped atcargo fmt --all --check; commit18d646capplies rustfmt 1.95 formatting to the privacy gate and its test. A fresh full matrix is required for the new head.CI follow-up: the first database rerun found a fixture clock race in the new #80 integration job: setup time could make the seeded current-day row newer than the test timestamp. Commit
a7870bdcaptures the timestamp immediately before seeding so the fixture and assertions share one clock.Final CI run
37705581476ona7870bd: PASS — Web, database integration tests, Rust Linux/macOS/Windows core and desktop, release scripts, and Greptile review. The database fixture clock adjustment and transient web build rerun are included in this green result.Latest-base update: merged
origin/mainat57f13c0after the prior green run; this adds the current release/orientation changes without conflicts. The final matrix is rerunning on the merge head.Final post-merge CI run
37708812793on989d746: PASS — Web, database integration tests, Rust Linux/macOS/Windows core and desktop, and release scripts. The branch is based on the latestorigin/main.Latest-base update: merged
origin/mainat0833b0aintod2b2d86without conflicts. This includes the current billing, desktop packaging, and Windows GUI changes; the full CI matrix is running on this merge head.Latest-base update: merged current
origin/mainat5e6026aintoe23b1d4without conflicts. The branch is now based on the current main tip; CI is rerunning on this final merge head.Desktop scrubber follow-up
Commit
4957039extends the desktop scrubber for email addresses, Convt and grouped license keys, bearer/authorization values, keyed credentials, JWT-shaped values, and long secret strings. The same scrubber runs over exception values, stack text, stack frames, and crash files. Tests build all sensitive samples from runtime parts and include a stack-frame case.fontconfigsystem library (fontconfig.pc). The pushed CI matrix is required for verification.Final scrub test fix: commit
c569826recognizes JWT values with ajwt=orjwt:label, which was the missing third token marker in the stack-frame test. No other behavior changed.Final privacy blocker fix: commit
cb1eb83handles whitespace-separatedAuthorization:,token:,Bearer, andBasicvalues, including scheme-plus-value forms. Runtime-built tests cover both message and stack text.Path privacy follow-up: commit
0fa44abnow consumes complete absolute paths through spaces on macOS, Windows, and Linux roots before applying credential redaction. Added runtime tests for all three path forms. The cited database failure was the pre-existing fixture-clock race; the timestamp fix is already in this branch and later database CI runs pass.CI follow-up: run
37718491726found only Clippy'smanual_pattern_char_comparisonwarning on the new path delimiter predicate. Commitb6f7aedapplies the lint-prescribed equivalent; no behavior changed.Created with GPT-6.1 Sol in T3 Code.