Skip to content

error-cleanup-checklist.md

Latest commit

 

History

History
69 lines (50 loc) · 9.28 KB

error-cleanup-checklist.md

File metadata and controls

69 lines (50 loc) · 9.28 KB

Error Cleanup Checklist

Tracking fixes for the top toast:show_error events identified via PostHog (last 30 days, prod builds only).

Already done in PR #13312 ("Fix bugs and improve error handling")

  • TypeError: undefined is not an object (i.type) — customHooks.svelte.ts now uses RTK Query unwrap() (272 events / 81 users)
  • erro_title typo in PostHog capture — error.ts:102
  • commiting typo — uncommittedService.svelte.ts:233 error string
  • shouldIgnoreThistError typo — parser.ts + showError.ts
  • Generate branch name disabled on empty branch — BranchHeaderContextMenu.svelte (~20 events/mo)
  • Rate-limit toast PostHog captures to 60/hour — toasts.ts (caps runaway spikes; mitigates most "Failed to fetch" storms)
  • 401 → actionable "Login token expired. Please log in to GitButler again." — httpClient.ts (120 events / 95 users)
  • SilentError for Octokit rate limiter — ratelimit.ts (~76 events)

Bugs to fix

  • Path is a directory — return empty FileInfo for directories in read_file_from_workspace (crates/gitbutler-repo/src/commands.rs:297) — 188 events / 14 users
  • Hunk not found while committing — skip unmatched hunks instead of throwing in uncommittedService.svelte.ts:233 — 56 events / 17 users
  • Failed to fetch diff eliminated — removed throw in getUnifiedDiff, widened return type to UnifiedDiff | null, callers already handle null via optional chaining; real IPC errors still surface via RTK unwrap() — 57 events / 11 users
  • keychain_notfound telemetry fix — annotate_keychain_error now wraps with Context::new_static(Code, stable_msg) for both SecretKeychainNotFound and MissingLoginKeychain (crates/but-secret/src/secret.rs) — 23 events / 21 users (telemetry only; UI already shows friendly text)

UX conversions (noisy but legitimate)

  • Generate commit message: info toast on no changes — swap showError → info showToast in macros.svelte.ts (covers both commit-message and branch-name generation paths) — 104 events / 48 users
  • WSL2 / UNC guidance as info toast — projectsService.ts:142-161 now uses showToast({style: "info"}) for both unsupported-path cases — 53 events / 37 users
  • install_cli osascript decline as info — Rust side tags status-1 with Code::CliInstallCancelled (but-action/src/cli.rs); GeneralSettings.svelte matches on that code and shows info toast — 32 events / 24 users
  • App update RO-filesystem: info toast with guidance — detect "Read-only file system" / EROFS in updater.ts:handleError and show info toast linking to downloads — 14 events / 5 users

Follow-ups from review

Concerns surfaced when reviewing the first round of fixes, addressed as separate commits on the same branch.

  • Silent whole-file commit risk — uncommittedService.svelte.ts pushed hunkHeaders: [] when every selected hunk in a file was stale, which the backend interprets as "commit whole file." Now we track stale-skip count per path and drop the file from the commit (plus an info toast) when every selection was stale. HIGH severity — potential data surprise.
  • Directory vs. empty-file ambiguity — introduced FileInfo::directory() as a semantic constructor for the directory case so the read path reads cleanly; did not add a dedicated marker field because no current consumer distinguishes directories from zero-byte files, and overloading mime_type with inode/directory would confuse the ImageDiff renderer that uses the field for data: URL building. If a future caller needs to differentiate, a proper is_directory: bool can be added then.
  • osascript cancel string-matching — added Code::CliInstallCancelled (errors.cli.install_cancelled), Rust side attaches it via Context::new_static, frontend matches on getUserErrorCode(err) === Code.CliInstallCancelled instead of the English message.
  • RO-filesystem pattern English-centric / Linux-centric — broadened to cover os error 30 (Linux EROFS numeric), os error 6032 (Windows ERROR_WRITE_PROTECT), and "write-protected" / "write protected" phrasing. Deliberately avoided bare "Permission denied" to prevent over-matching.
  • Keychain annotation Linux-only — renamed annotate_linux_keychain → annotate_keychain_error and attached a stable "System keychain access failed" label with Code::Unknown for macOS/Windows + unmatched Linux paths, so PostHog aggregates these rather than bucketing every localized error separately.

Deferred (nice-to-haves from review, not actioned)

  • Disable AI-generate button when there are no changes — would require reactive plumbing from selection/diff state up to the toolbar. The info-toast conversion already removes the error noise; this is a UI polish follow-up.
  • Modal/inline messaging for WSL2/UNC guidance — info toast conveys the guidance; a modal would be more prominent but is a UX decision for a separate design pass.
  • Dedicated Code for macOS/Windows keychain — those platforms always have a default keychain, so a bespoke Code has no known user-facing remediation to tie to. Revisit if telemetry shows a cluster.

query:error path (high-volume silent RTK Query errors)

Context: query:error captures ~1.6M events / 14 days / 4.2k users — roughly 100× the volume of toast:show_error. Investigation (2026-04-16) showed massive single-user spam loops (one user repeatedly hitting a broken stacks / list_reviews call fires tens of thousands of times) and wide command-not-found fallout (irc_*, forge_provider) that never surfaces as a toast.

  • Rate limit + per-key dedup on emitQueryError — error.ts adds a 60-minute rolling window with an overall 200-event cap and a 5-event cap per (command, error_title) pair. Also skips SilentError defensively. Expected to cut query:error volume by ~100× without losing signal.
  • Forward command + actionName to capture payload — customHooks.svelte.ts now passes the RTK endpoint context into emitQueryError, so PostHog can group by command instead of string-parsing API error: (cmd) out of error_message. This surfaces clusters like per-project stacks/list_reviews that were previously fragmented across one-user buckets with project IDs embedded in the message.
  • erro_title typo confirmed fixed in source — grep shows only error_title in all four capture sites (error.ts, customHooks.svelte.ts, toasts.ts, posthog.ts). Deployed 1.360.2 still emits the typo because the fix rode into nightly but hasn't shipped in a stable release yet; next release will clear it. No code change needed.

Round 2 — top untracked errors (24h snapshot, 2026-04-17)

  • "Expected to be in edit mode" as Unhandled exception — 90 events / 36 users in 24h. Already fixed in prior work but still firing; confirmed shipped — residual is old-build tail. No code change needed.
  • Git hook output shown as error toast — ~20 events / ~15 users. Hook failures were surfacing with generic "Error" title (from Error.name) or "Unhandled exception" (from unhandled promise rejections in commitDropHandler.ts). Fix: hooksService.ts now throws errors with name = "Git hook failed" for better PostHog grouping; commitDropHandler.ts wraps hook calls in try/catch with showError("Git hook failed", err) instead of letting them propagate as unhandled rejections.
  • "Git push failed" toast shows raw command params — 73 events / 31 users. backendQuery.ts was prepending command: ...\nparams: {JSON} to every backend error message, burying the actual error. Fix: removed the command/params prefix — the command name is already in the error name field (API error: (push_stack)), and the actual error message now surfaces cleanly in the toast.
  • 401 Unauthorized under generic "Error" title — 8 events / 7 users. Some servers/proxies return a non-401 status code but with {"error":"401 Unauthorized"} in the body, bypassing the status-code check. Fix: httpClient.ts parseResponseJSON now also checks the response body for "401 Unauthorized" in the >= 400 branch and shows the friendly login-expired message.
  • "Failed to amend commit: noEffectiveChanges" as error — 4–7 events / 1–2 users. Fix: stackEndpoints.ts commitAmend.transformResponse now detects when all rejections are noEffectiveChanges, shows an info toast ("No changes to amend"), and throws SilentError to suppress the error toast while still signaling mutation failure.

Not addressed (deferred / out of scope)

  • Linux auto-updater "invalid updater binary format" (178 events / 103 users) — Tauri/distro issue, needs separate effort
  • Various single-user repeat errors (set_project_active, Windows R:/ path, etc.) — environment-specific
  • Expected to be in edit mode (2,811 events / 353 users) — already resolved prior to this session
  • IRC command-not-found spam (irc_get_file_message_reactions, irc_get_all_commit_reactions — ~615k events / 1.8k users) — feature-flag gating on the call sites; deliberately out of scope for this round. Rate-limit changes above will already dedupe this down to ~5 events/user/hour per command.
  • forge_provider not found / ACL-blocked (~67k events / ~5k users) — same class as IRC; new rate limit handles it.