Demote non-actionable report_error! calls in terminal/view.rs#13857
Open
warp-dev-github-integration[bot] wants to merge 3 commits into
Open
Demote non-actionable report_error! calls in terminal/view.rs#13857warp-dev-github-integration[bot] wants to merge 3 commits into
warp-dev-github-integration[bot] wants to merge 3 commits into
Conversation
Audit of all report_error! calls in app/src/terminal/view.rs to reduce Sentry noise, per .agents/skills/logging-and-error-reporting/SKILL.md. Only report_error! (and panics) create Sentry issues, so calls that don't represent actionable engineering problems should be plain logs instead. Demote 8 calls to log::warn! (nothing for an engineer to fix): - 4x persistence ModelEvent channel-send failures (UpdateBlockAgentViewVisibility x3, UpdateFinishedCommand): a send error means the receiver is gone during app teardown, a race rather than a bug. - 2x "end_selection dispatched with no pending selection": a benign UI event-ordering condition (end without a matching begin). - "Cannot insert PR comments: not in a git repository": an expected environmental precondition. - "No working directory found for terminal session": an expected environmental condition (e.g. remote / not-yet-bootstrapped sessions). Keep the remaining 13 report_error! calls: they flag genuine invariant violations (missing conversation/session that should exist), settings persistence failures, unexpected agent-view entry failures, and a deliberately-captured unknown OS permission error. Co-Authored-By: Warp <agent@warp.dev>
Remove the explanatory comments added alongside the demoted log::warn! calls, per in-thread review feedback. The log-level changes are unchanged. Co-Authored-By: Warp <agent@warp.dev>
An end-selection dispatched without a matching begin is an invariant violation the code handles locally, so log it at error! rather than warn! (still not a Sentry issue via report_error!). Per in-thread review feedback. Co-Authored-By: Warp <agent@warp.dev>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Audits every
report_error!call inapp/src/terminal/view.rsto reduce Sentry noise, per.agents/skills/logging-and-error-reporting/SKILL.md. Onlyreport_error!(and panics) create Sentry issues;log::*at Info+ becomes a breadcrumb. Calls that don't represent an actionable engineering problem — network/teardown races, expected fallbacks, gracefully-handled environmental conditions — should be plain logs, not Sentry events.Demoted 8 calls to
log::warn!(nothing for an engineer to fix):ModelEventchannel-send failures (UpdateBlockAgentViewVisibility×3,UpdateFinishedCommand). Asenderror means the receiver is gone during app teardown — a race, not a bug."end_selection dispatched with no pending selection"— a benign UI event-ordering condition (an end without a matching begin)."Cannot insert PR comments: not in a git repository"— an expected environmental precondition."No working directory found for terminal session"— an expected environmental condition (e.g. remote / not-yet-bootstrapped sessions).Kept the remaining 13
report_error!calls — they flag genuine problems worth Sentry attention:set_value) failures (alias expansion, codebase-index speedbump banner ×2, agent-mode setup banner, notifications setting ×2).Linked Issue
N/A — requested via the factory-client bug-triage Slack thread (linked below).
Testing
./script/format --checkpasses.cargo clippy -p warp --all-targets --tests -- -D warningspasses (CARGO_BUILD_JOBS=1).No new automated test: this is a pure observability/log-level change with no behavioral difference (the error paths still early-return exactly as before), so there is no behavior to assert in a regression test. Not user-visible, so no UI verification applies.
Agent Mode