Reduce Sentry noise from non-actionable report_error! in terminal view#13909
Open
warp-dev-github-integration[bot] wants to merge 1 commit into
Open
Reduce Sentry noise from non-actionable report_error! in terminal view#13909warp-dev-github-integration[bot] wants to merge 1 commit into
warp-dev-github-integration[bot] wants to merge 1 commit into
Conversation
Audit all report_error! calls in app/src/terminal/view.rs against the logging-and-error-reporting skill and demote the ones that don't represent actionable engineering issues to log::warn!. Demoted (fire on expected/recoverable/handled conditions, not bugs): - 4 model-event channel sends (mpsc SendError fires on receiver teardown during shutdown). - The "not in a git repository" guard when the agent requests PR-comment insertion (an expected precondition, gracefully handled). - 6 best-effort setting-persistence failures (the underlying write in write_to_preferences is already best-effort/ignored). Kept as report_error! (genuine invariant violations / unexpected failures): missing conversation/session lookups, agent-view entry failures, the cleanup-with-other-blocks invariant, end_selection-with-no-selection, the unknown notification-permission error, and the missing working directory. CHANGELOG-BUG-FIX: Co-Authored-By: Oz <oz-agent@warp.dev>
Contributor
|
@warp-dev-github-integration[bot] I'm starting a first review of this pull request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
Contributor
There was a problem hiding this comment.
Overview
This PR demotes non-actionable terminal view report_error! sites to log::warn! while preserving the existing control flow and user-visible behavior.
Concerns
- No blocking correctness, security, or spec-drift concerns found in the attached diff.
Verdict
Found: 0 critical, 0 important, 0 suggestions
Approve
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
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.rsagainst thelogging-and-error-reportingskill to reduce Sentry noise, demoting calls that don't represent actionable engineering issues tolog::warn!.report_error!is the only form that creates a Sentry issue, so it should be reserved for genuine bugs (invariant violations / states that shouldn't be possible); expected, recoverable, or environmental failures belong atlog::warn!.21 call sites were reviewed. 11 demoted to
log::warn!; 10 kept asreport_error!.Demoted to
log::warn!(fire on expected / recoverable / handled conditions)sender.send(persistence::ModelEvent::…)failures forUpdateBlockAgentViewVisibility(×3) andUpdateFinishedCommand. An mpscSendErrorfires when the receiver has been dropped — an expected teardown/shutdown race, not a bug. These typed errors aren't registered as non-actionable, so today they hit Sentry on every teardown.set_value(...)failures for alias-expansion enable, the two codebase-index speedbump-banner dismissals, the Agent-Mode setup-banner-shown flag, and the two notifications-setting writes. The underlying write inwrite_to_preferencesis already best-effort (its result is deliberately ignored), the failures are recoverable/handled, and there is existing in-file precedent (log::warn!for the MCP-execution-pathset_valuefailure).Kept as
report_error!(genuine invariant violations / unexpected failures)Missing conversation lookup for the usage footer; missing session after a bootstrap notification; the two agent-view entry failures for passive code diffs; the inline agent-view tag-in failure; the "clean up conversation that still has other blocks" invariant; the two
end_selection dispatched with no pending selectionguards; the unknown notification-permission error (the explicit unknown-error branch); and the missing working directory for a local terminal session.Testing
./script/formatpasses.cargo clippy -p warp --all-targets --tests -- -D warningspasses.No new automated test: this change only reclassifies log/telemetry severity levels — there is no observable runtime behavior change to assert on (the surrounding code already handles each of these error paths identically).
Agent Mode
Conversation: https://staging.warp.dev/conversation/10f04aa7-3bb8-45d5-ac0f-8c462c8caab3
Run: https://oz.staging.warp.dev/runs/019f71d6-0673-7ea2-a5cc-4e186941947f
This PR was generated with Oz.