Repository navigation
Type incoming request ids as Int64, not Int (fixes 32-bit Julia) - #127
Merged
Merged
Conversation
The JSON parser returns integer request ids as Int64. On 32-bit Julia
`Int === Int32`, so storing the id of the very first incoming request in
`cancellation_sources` threw `MethodError: Cannot convert Int64 to
Union{Int32, String}`, which killed the read task with "Read task failed".
`Request.id` and `no_longer_needed_cancellation_sources` had the same
problem.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
`only` needs Julia 1.4, and CI tests back to 1.0. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #127 +/- ##
==========================================
- Coverage 92.02% 91.46% -0.56%
==========================================
Files 5 5
Lines 539 539
==========================================
- Hits 496 493 -3
- Misses 43 46 +3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
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.
This PR was opened by Claude (Claude Code) on David's behalf.
Problem
On 32-bit Julia, the julia-vscode language server dies on its first request. Crash telemetry has reported this since 1.219.2 (4 users, all
i686-w64-mingw32):The JSON parser returns integer request ids as
Int64. The id-keyed slots were typed withInt, and on 32-bit JuliaInt === Int32.convert(Union{Int32,String}, ::Int64)has no method (unlikeUnion{Nothing,Int32}, whichconverthandles), sox.cancellation_sources[id] = cancel_sourcein the read task threw, and the endpoint went tostatus_errored.Fix
These three slots now use
Int64instead ofInt:Request.idJSONRPCEndpoint.cancellation_sourcesJSONRPCEndpoint.no_longer_needed_cancellation_sourcesOther id handling I checked:
outstanding_requestsis keyed byString, and our own outgoing ids are UUID strings. A response with an integer id is looked up withget, which does not convert, so an unknown integer id still gives the existing "unknown request id"TransportError, not aMethodError.$/cancelRequestlookup and thedelete!of finished ids also useget/delete!, so neither converts.send_success_response/send_error_responseechooriginal_request.idback unchanged.typed.jlandinterface_def.jlonly compare the id withnothing.Tests
The new
test/test_request_ids.jlhas two test items:Int64id outside theInt32range, and the value goes through each of them unconverted.typemax(Int32) + 5to a running endpoint, cancels it with$/cancelRequest, and gets the response back with the sameInt64id. A further message then confirms that the finished request's cancellation source was dropped and that the endpoint is still running. The waits time out, so on a broken build the test fails instead of hanging.CI includes
~x86(32-bit) legs, so the new tests guard against this from now on. Before pushing, I also ran the full suite locally with juliaup's1.12~x86channel (Sys.WORD_SIZE == 32):main+ new testsTransportError("Read task failed", MethodError(convert, (Union{Int32, String}, 2147483652)))The first push used
only, which needs Julia 1.4, so the 1.0–1.3 legs failed. The second commit replaces it, and the suite now passes locally on Julia 1.0.5 (622/622) and on 32-bit 1.12.7 (622/622).On 64-bit Julia
Int64 === Int, so this PR changes nothing there.Downstream
This removes only the first 32-bit failure. Once the language server gets past its first request on 32-bit Julia, more failures of the same kind may show up downstream. A quick look shows protocol types that will fail the same way whenever a peer sends an integer:
ProgressTokenfields (workDoneToken/partialResultToken/token::Union{Int,String[,Missing]}). VS Code normally sends string tokens.src/protocol/types.jl(id::Union{Int,String},moduleId::Union{Missing,Int,String}).None of these have shown up in telemetry yet. They are not changed here.
🤖 Generated with Claude Code