Add JSON.jl 1.x compatibility

.github/workflows/juliaci.yml

@@ -15,3 +15,32 @@ jobs:
     permissions: write-all
     secrets:
       codecov_token: ${{ secrets.CODECOV_TOKEN }}
+
+  json-compat:
+    name: JSON ${{ matrix.json-version }}
+    # Only run where `github.ref` points at the code under test. On
+    # `issue_comment` it points at the default branch, so the job would test
+    # main while reporting its status on the pull request.
+    if: >-
+      github.event_name == 'push' || github.event_name == 'pull_request' ||
+      (github.event_name == 'workflow_dispatch' && github.event.inputs.feature == 'LintAndTest')
+    runs-on: ubuntu-latest
+    strategy:
+      fail-fast: false
+      matrix:
+        json-version: ["0.20", "0.21", "1.7"]
+    env:
+      DEBUGADAPTER_EXPECTED_JSON_VERSION: ${{ matrix.json-version }}
+    steps:
+      - uses: actions/checkout@v4
+      - uses: julia-actions/setup-julia@v2
+        with:
+          version: "1"
+      - uses: julia-actions/cache@v2
+      - name: Install JSON ${{ matrix.json-version }}
+        # Pin so that the resolve `Pkg.test` runs for the test-only dependencies
+        # cannot quietly move JSON to a different version. The test suite
+        # asserts the version it actually loaded against the env var above.
+        run: |
+          julia --project=. -e 'using Pkg; Pkg.add(PackageSpec(name="JSON", version="${{ matrix.json-version }}")); Pkg.pin("JSON"); Pkg.status()'
+      - uses: julia-actions/julia-runtest@v1

Project.toml

@@ -15,7 +15,7 @@ TestItemRunner = "f8b46487-2199-4994-9208-9a1283c18c0a"
 Test = "8dfed614-e22c-5e08-85e1-65c5234f0b40"
 
 [compat]
-JSON = "0.20, 0.21"
+JSON = "0.20, 0.21, 1"
 julia = "1"
 JuliaInterpreter = "0.8.5, 0.9, 0.10, 0.11"
 

src/DAPRPC/core.jl

@@ -1,15 +1,19 @@
 struct DAPError <: Exception
     msg::AbstractString
+    code::Int
+    data::Any
 end
 
+DAPError(msg::AbstractString) = DAPError(msg, -32603, nothing)
+
 mutable struct DAPEndpoint{IOIn <: IO,IOOut <: IO}
     pipe_in::IOIn
     pipe_out::IOOut
 
     out_msg_queue::Channel{Any}
     in_msg_queue::Channel{Any}
 
-    outstanding_requests::Dict{String,Channel{Any}}
+    outstanding_requests::Dict{Int,Channel{Any}}
 
     err_handler::Union{Nothing,Function}
 
@@ -22,7 +26,7 @@
 end
 
 DAPEndpoint(pipe_in, pipe_out, err_handler = nothing) =
-    DAPEndpoint(pipe_in, pipe_out, Channel{Any}(Inf), Channel{Any}(Inf), Dict{String,Channel{Any}}(), err_handler, :idle, nothing, nothing, 0)
+    DAPEndpoint(pipe_in, pipe_out, Channel{Any}(Inf), Channel{Any}(Inf), Dict{Int,Channel{Any}}(), err_handler, :idle, nothing, nothing, 0)
 
 function write_transport_layer(stream, response)
     response_utf8 = transcode(UInt8, response)
@@ -102,7 +106,7 @@
                 break
             end
 
-            message_dict = JSON.parse(message)
+            message_dict = _parse_json(message)
 
             if message_dict["type"] == "request" || message_dict["type"] == "event"
                 try
@@ -116,7 +120,7 @@
                 end
             elseif message_dict["type"] == "response"
                 # This must be a response
-                id_of_request = message_dict["request_seq"]
+                id_of_request = Int(message_dict["request_seq"])
 
                 channel_for_response = x.outstanding_requests[id_of_request]
                 put!(channel_for_response, message_dict)
@@ -151,7 +155,7 @@
 
     message = Dict("seq" => x.seq, "type" => "event", "event" => method, "body" => params)
 
-    message_json = JSON.json(message)
+    message_json = _json(message)
 
     put!(x.out_msg_queue, message_json)
 
@@ -170,7 +174,7 @@
     response_channel = Channel{Any}(1)
     x.outstanding_requests[x.seq] = response_channel
 
-    message_json = JSON.json(message)
+    message_json = _json(message)
 
     put!(x.out_msg_queue, message_json)
 
@@ -179,13 +183,29 @@
     if response["success"]==true
         return response["body"]
     elseif response["success"]==false
-        error_message = response["message"]
-        throw(DAPError(error_message))
+        throw(dap_error(response))
     else
         throw(DAPError("ERROR AT THE TRANSPORT LEVEL"))
     end
 end
 
+# The structured `Message` in `body.error` is optional in DAP, and only it
+# carries the error id and variables, so fall back to the short `message` the
+# response is required to have when the peer did not send one.
+function dap_error(response)
+    fallback = get(response, "message", "The request failed.")
+
+    body = get(response, "body", nothing)
+    error_message = body isa AbstractDict ? get(body, "error", nothing) : nothing
+    error_message isa AbstractDict || return DAPError(fallback)
+
+    return DAPError(
+        get(error_message, "format", fallback),
+        Int(get(error_message, "id", -32603)),
+        get(error_message, "variables", nothing),
+    )
+end
+
 function get_next_message(endpoint::DAPEndpoint)
     check_dead_endpoint!(endpoint)
 
@@ -194,7 +214,7 @@
     return msg
 end
 
 function Base.iterate(endpoint::DAPEndpoint, state = nothing)
     check_dead_endpoint!(endpoint)
 
     try
@@ -215,7 +235,7 @@
 
     response = Dict("seq" => endpoint.seq, "type" => "response", "request_seq" => original_request["seq"], "success" => true, "command" => original_request["command"], "body" => result)
 
-    response_json = JSON.json(response)
+    response_json = _json(response)
 
     put!(endpoint.out_msg_queue, response_json)
 end
@@ -225,9 +245,23 @@
 
     endpoint.seq += 1
 
-    response = Dict("seq" => endpoint.seq, "request_seq" => original_request["seq"], "error" => Dict("code" => code, "message" => message, "data" => data))
+    error_body = Dict{String,Any}("id" => code, "format" => message)
+    # `isnothing` postdates the Julia versions this package supports.
+    if data !== nothing
+        error_body["variables"] = data
+    end
 
-    response_json = JSON.json(response)
+    response = Dict(
+        "seq" => endpoint.seq,
+        "type" => "response",
+        "request_seq" => original_request["seq"],
+        "success" => false,
+        "command" => original_request["command"],
+        "message" => message,
+        "body" => Dict("error" => error_body),
+    )
+
+    response_json = _json(response)
 
     put!(endpoint.out_msg_queue, response_json)
 end

src/DAPRPC/interface_def.jl

@@ -1,24 +1,24 @@
 abstract type Outbound end
 
-function JSON.Writer.CompositeTypeWrapper(t::Outbound)
-    fns = collect(fieldnames(typeof(t)))
-    dels = Int[]
-    for i = 1:length(fns)
-        f = fns[i]
-        if getfield(t, f) isa Missing
-            push!(dels, i)
-        end
-    end
-    deleteat!(fns, dels)
-    JSON.Writer.CompositeTypeWrapper(t, Tuple(fns))
-end
-
+# Optional DAP fields are represented as `missing` and must be omitted from the
+# wire format entirely, rather than serialized as `null`.
+#
+# We lower to a `Dict{String,Any}` rather than to a `NamedTuple` of the present
+# fields: a `NamedTuple` would mint a distinct concrete type for every subset of
+# non-missing fields, forcing the JSON writer to compile a fresh specialization
+# for each shape a type happens to take. Types like `StackFrame` and `Variable`
+# are sent with varying field sets while stepping, so that cost is paid over and
+# over during an interactive session. A `Dict` also serializes as `{}` when no
+# field is present, which every supported JSON.jl version handles.
 function JSON.lower(a::Outbound)
-    if nfields(a) > 0
-        JSON.Writer.CompositeTypeWrapper(a)
-    else
-        nothing
+    nfields(a) == 0 && return nothing
+
+    result = Dict{String,Any}()
+    for field in fieldnames(typeof(a))
+        value = getfield(a, field)
+        ismissing(value) || (result[String(field)] = value)
     end
+    return result
 end
 
 function field_allows_missing(field::Expr)
@@ -27,7 +27,7 @@
     any(i -> i == :Missing, field.args[2].args)
 end
 
 function field_type(field::Expr, typename::String)
     if field.args[2] isa Expr && field.args[2].head == :curly && field.args[2].args[1] == :Union
         if length(field.args[2].args) == 3 && (field.args[2].args[2] == :Missing || field.args[2].args[3] == :Missing)
             return field.args[2].args[2] == :Missing ? field.args[2].args[3] : field.args[2].args[2]
@@ -60,7 +60,7 @@
         end
         ) : nothing)
 
-        function $tname(dict::Dict)
+        function $tname(dict::AbstractDict)
         end
     end
 

src/DAPRPC/jsoncompat.jl

@@ -0,0 +1,11 @@
+# Everything version-sensitive about our use of JSON.jl lives here, so that the
+# assumptions we make about a given JSON.jl release are visible in one place.
+#
+# Read side: JSON 1.x parses objects into `JSON.Object` by default, while the
+# rest of the package expects plain `Dict{String,Any}`, recursively.
+_parse_json(value) = JSON.parse(value; dicttype=Dict{String,Any})
+
+# Write side: we rely on `JSON.lower` (see `interface_def.jl`) being honored for
+# our own types, and on `AbstractDict` serializing as a JSON object. Both hold
+# for every version allowed by `[compat]`.
+_json(value) = JSON.json(value)

src/DAPRPC/packagedef.jl

@@ -1,5 +1,6 @@
 export DAPEndpoint, send_notification, send_request, send_success_response, send_error_response
 
+include("jsoncompat.jl")
 include("core.jl")
 include("typed.jl")
 include("interface_def.jl")

test/test_daprpc.jl

@@ -0,0 +1,121 @@
+@testsnippet DapEndpointPair begin
+    import Sockets
+
+    """
+        endpoint_pair() -> (client, adapter, cleanup)
+
+    Two `DAPEndpoint`s wired to each other over a loopback socket, both already
+    running, plus a function that tears the pair down.
+    """
+    function endpoint_pair()
+        port, server = Sockets.listenany(Sockets.localhost, 0)
+        client_conn = Sockets.connect(Sockets.localhost, port)
+        adapter_conn = Sockets.accept(server)
+
+        client = DebugAdapter.DAPRPC.DAPEndpoint(client_conn, client_conn)
+        adapter = DebugAdapter.DAPRPC.DAPEndpoint(adapter_conn, adapter_conn)
+        run(client)
+        run(adapter)
+
+        return client, adapter, () -> begin
+            close(client)
+            close(adapter)
+            close(server)
+        end
+    end
+end
+
+@testitem "DAPRPC routes responses back to the caller" setup=[DapEndpointPair] begin
+    client, adapter, cleanup = endpoint_pair()
+
+    try
+        succeeded = Ref{Any}(nothing)
+        request_task = @async succeeded[] = DebugAdapter.DAPRPC.send_request(client, "runInTerminal", Dict{String,Any}())
+
+        incoming = DebugAdapter.DAPRPC.get_next_message(adapter)
+        @test incoming["type"] == "request"
+        @test incoming["command"] == "runInTerminal"
+
+        DebugAdapter.DAPRPC.send_success_response(adapter, incoming, Dict{String,Any}("processId" => 17))
+        wait(request_task)
+        @test succeeded[]["processId"] == 17
+
+        failure = Ref{Any}(nothing)
+        failing_task = @async try
+            DebugAdapter.DAPRPC.send_request(client, "evaluate", Dict{String,Any}())
+        catch err
+            failure[] = err
+        end
+
+        incoming = DebugAdapter.DAPRPC.get_next_message(adapter)
+        DebugAdapter.DAPRPC.send_error_response(adapter, incoming, 42, "boom", nothing)
+        wait(failing_task)
+        @test failure[] isa DebugAdapter.DAPRPC.DAPError
+        @test failure[].msg == "boom"
+        # The whole `Message` survives the round trip, not just its text.
+        @test failure[].code == 42
+        @test failure[].data === nothing
+
+        with_variables = Ref{Any}(nothing)
+        variables_task = @async try
+            DebugAdapter.DAPRPC.send_request(client, "evaluate", Dict{String,Any}())
+        catch err
+            with_variables[] = err
+        end
+
+        incoming = DebugAdapter.DAPRPC.get_next_message(adapter)
+        DebugAdapter.DAPRPC.send_error_response(adapter, incoming, 7, "no such variable", Dict("name" => "x"))
+        wait(variables_task)
+        @test with_variables[].code == 7
+        @test with_variables[].data == Dict("name" => "x")
+    finally
+        cleanup()
+    end
+end
+
+@testitem "DAPRPC error responses are valid DAP" begin
+    import Sockets
+
+    # A bare socket on the far end, so that the exact bytes `send_error_response`
+    # puts on the wire can be inspected rather than an endpoint's view of them.
+    port, server = Sockets.listenany(Sockets.localhost, 0)
+    peer = Sockets.connect(Sockets.localhost, port)
+    conn = Sockets.accept(server)
+
+    endpoint = DebugAdapter.DAPRPC.DAPEndpoint(conn, conn)
+    run(endpoint)
+
+    try
+        request = Dict{String,Any}("seq" => 3, "type" => "request", "command" => "evaluate")
+        DebugAdapter.DAPRPC.send_error_response(endpoint, request, 42, "boom", nothing)
+
+        header = readline(peer)
+        @test startswith(header, "Content-Length:")
+        length_of_body = parse(Int, strip(split(header, ':')[2]))
+        readline(peer)  # the blank line between header and body
+        response = DebugAdapter.DAPRPC._parse_json(String(read(peer, length_of_body)))
+
+        @test response["type"] == "response"
+        @test response["success"] === false
+        @test response["command"] == "evaluate"
+        @test response["request_seq"] == 3
+        @test response["message"] == "boom"
+        @test response["body"]["error"]["id"] == 42
+        @test response["body"]["error"]["format"] == "boom"
+        # `data` was `nothing`, and DAP has no null-valued `variables`.
+        @test !haskey(response["body"]["error"], "variables")
+    finally
+        close(endpoint)
+        close(peer)
+        close(server)
+    end
+end
+
+@testitem "DAPRPC tolerates error responses without a structured message" begin
+    # `body.error` is optional in DAP, so a peer may send only the short
+    # `message`, and the id and variables are then simply not available.
+    minimal = DebugAdapter.DAPRPC.dap_error(Dict{String,Any}("success" => false, "message" => "cancelled"))
+    @test minimal.msg == "cancelled"
+    @test minimal.code == -32603
+    @test minimal.data === nothing
+end