Open
Conversation
…verification tests
1. Removed unsafe .Value call (line 455) - The original code accessed wf.Value before the match to check if wf was Some. This caused crashes when the workspace folder wasn't found. 2. Moved wfPathToUri into the Some wf, Some doc branch (line 510) - The calculation now only happens when we're certain wf is Some. 3. Added .cshtml extension check (line 460) - The Some wf, None branch now only processes files as Razor/cshtml if they actually have a .cshtml extension, instead of treating ALL unfound documents as cshtml files. 4. Safe handling of wf.Solution (lines 461-466) - Instead of the unsafe wf.Solution.Value, the code now pattern matches on wf.Solution to safely handle the case where the solution hasn't loaded yet. 5. Added retry logic for unfound documents (lines 464-465, 504-505) - When a document isn't found (due to timing or solution not being fully loaded), the code now posts PushDiagnosticsDocumentBacklogUpdate and PushDiagnosticsProcessPendingDocuments to rebuild the backlog and retry later. 6. Added continuation for unfound .cshtml documents (line 495) - When a .cshtml document isn't found as a Razor document, processing continues with the next document. All 67 tests pass, including the two previously failing tests: - testPushDiagnosticsWork - testDiagnoseCommandWorks
…-fix-tests-on-main
There was a problem hiding this comment.
Pull request overview
This PR fixes test failures on main and addresses issue razzmatazz#306 by improving error handling in the ProgressReporter and fixing a critical bug in ServerState's diagnostic processing logic. The PR also adds comprehensive unit tests for the ProgressReporter to ensure exceptions from unsupported client methods are handled gracefully.
Changes:
- Added exception handling in ProgressReporter.Begin to gracefully handle clients that don't support window/workDoneProgress/create
- Fixed a bug in ServerState where
wf.Valuewas accessed before null checking, which could cause exceptions - Improved .cshtml file handling with explicit extension checking and solution loading state management
- Added comprehensive unit tests for ProgressReporter error handling scenarios
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| tests/CSharpLanguageServer.Tests/ProgressReporterTests.fs | New test file validating exception handling in ProgressReporter |
| tests/CSharpLanguageServer.Tests/CSharpLanguageServer.Tests.fsproj | Added reference to new ProgressReporterTests.fs file |
| src/CSharpLanguageServer/Lsp/ProgressReporter.fs | Added try-catch to handle exceptions from clients that don't support progress reporting |
| src/CSharpLanguageServer/State/ServerState.fs | Fixed unsafe Option.Value access, added .cshtml extension check, and improved retry logic for unloaded solutions |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…eporting Checks the client's window.workDoneProgress capability before attempting to create progress tokens. If the capability is not advertised or is false, progress reporting is silently skipped. Changes: - ProgressReporter.fs: Add ClientCapabilities parameter to constructor; check Window.WorkDoneProgress capability before calling WindowWorkDoneProgressCreate; remove try-catch exception handling - Workspace.fs: Pass ClientCapabilities to ProgressReporter - ServerState.fs: Pass state.ClientCapabilities to workspace loading - Diagnostics.fs: Pass emptyClientCapabilities to ProgressReporter - Tooling.fs: Add Window.WorkDoneProgress = true to test client capabilities - ProgressReporterTests.fs: Replace exception-based tests with capability-based tests that verify WindowWorkDoneProgressCreate is only called when supported
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.
I merged both fixes into a single branch, build the solution and installed the cshap-lsp global tool into the dev container and configured the claude code csharp-lsp. I managed to invoke it and got it to interrogate a project I'm working on: