Skip to content

fix(lsp): log retried transient errors at warn, not error - #401

Merged
bug-ops merged 2 commits into
mainfrom
fix/retried-transient-lsp-errors
Sep 6, 2026
Merged

fix(lsp): log retried transient errors at warn, not error#401
bug-ops merged 2 commits into
mainfrom
fix/retried-transient-lsp-errors

Conversation

@bug-ops

@bug-ops bug-ops commented Sep 6, 2026

Copy link
Copy Markdown
Owner

Summary

  • message_loop_inner logged every LSP JSON-RPC error response at error! unconditionally, before LspClient::request's retry logic decided whether -32802 (ServerCancelled) or an allowlisted -32801 (ContentModified) would be retried. A request that was retried and then succeeded still emitted a spurious ERROR line, causing false-positive alerts in log-based monitoring.
  • The severity decision now lives in request(), the only place that knows whether a retry will follow: warn! on each retry attempt, error! only on retry exhaustion or a non-retryable error/method. All caller-visible error! lines share one consistent "LSP error response: ..." prefix, so log-grep monitoring keeps working for exhaustion cases too.
  • Promoted the level-aware tracing capture test helper into the shared test_lsp module, replacing three message-only duplicates in transport.rs, lib.rs, and bridge/notifications.rs.

Closes #392

Test plan

  • cargo +nightly fmt --all -- --check
  • cargo clippy --all-targets --all-features --workspace -- -D warnings
  • cargo nextest run --workspace --all-features --lib --bins (798 passed, 1 skipped)
  • RUSTDOCFLAGS="-D warnings" cargo doc --no-deps --all-features
  • New regression tests: retried-and-recovered error does not log ERROR; retry exhaustion logs exactly one ERROR sharing the shared prefix; non-retryable error still logs ERROR on first attempt

@github-actions github-actions Bot added rust Rust code changes testing Test-related changes mcpls-core mcpls-core crate changes documentation Improvements or additions to documentation labels Sep 6, 2026
@bug-ops
bug-ops force-pushed the fix/retried-transient-lsp-errors branch from 1f467e8 to 8e40fda Compare September 6, 2026 11:49
@bug-ops
bug-ops enabled auto-merge (squash) September 6, 2026 11:49
message_loop_inner logged every LSP error response at error!
unconditionally, before the retry logic in LspClient::request decided
whether -32802 (ServerCancelled) or an allowlisted -32801
(ContentModified) would be retried. A request that was retried and
succeeded still emitted a spurious ERROR line, causing false-positive
alerts in log-based monitoring.

Move the severity decision into request(), the only place that knows
whether a retry will follow: warn! on each retry attempt, error! only
on retry exhaustion or a non-retryable error. All caller-visible error!
lines now share one consistent "LSP error response: ..." prefix so
log-grep monitoring keeps working for exhaustion cases too.

Promote the level-aware tracing capture test helper into the shared
test_lsp module, replacing three message-only duplicates in transport,
lib, and bridge/notifications.
@bug-ops
bug-ops force-pushed the fix/retried-transient-lsp-errors branch from 8e40fda to 19f5127 Compare September 6, 2026 11:52
@bug-ops
bug-ops merged commit c724482 into main Sep 6, 2026
28 checks passed
@bug-ops
bug-ops deleted the fix/retried-transient-lsp-errors branch September 6, 2026 11:57
@bug-ops bug-ops mentioned this pull request Sep 6, 2026
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation mcpls-core mcpls-core crate changes rust Rust code changes testing Test-related changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

retried transient LSP errors (ContentModified/ServerCancelled) log at ERROR severity before the retry succeeds

1 participant