Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix non-sending of error response on unsupported methods #71

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Fix non-sending of error response on unsupported methods #71

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
160e271
Fix missing response on unknown method
Seelengrab Oct 17, 2022
698e905
Name unsupported methods in errors
Seelengrab Oct 17, 2022
9682a93
Add test for errors on wrongly requested methods
Seelengrab Oct 17, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion src/typed.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ function dispatch_msg(x::JSONRPCEndpoint, dispatcher::MsgDispatcher, msg)
dispatcher._currentlyHandlingMsg = true
try
method_name = msg["method"]
is_request = haskey(msg, "id")
handler = get(dispatcher._handlers, method_name, nothing)
if handler !== nothing
param_type = get_param_type(handler.message_type)
Expand All @@ -78,7 +79,11 @@ function dispatch_msg(x::JSONRPCEndpoint, dispatcher::MsgDispatcher, msg)
end
end
else
error("Unknown method $method_name.")
if is_request # even invalid requests MUST get a response - see Section 5 of the JSON RPC specification.
send_error_response(x, msg, -32601, "Unknown method '$method_name'.", nothing)
end
# TODO: ignoring notifications is legal, so there's no need to crash here on unknown things. However, removing this requires downstream support.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Support in LS.jl?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's in reference to something about the VSCode extension having built-in support for LS.jl crashing, for telemetry reasons (or so I recall from one of the issues on the LS.jl repo). I'm not familiar with how that works & interacts, but the assumption that the JSONRPC call throws an error is built into LS.jl, yes. Keeping the throwing behavior would need to be moved there, or changed to some other way entirely.

error("Unknown method '$method_name'.")
end
finally
dispatcher._currentlyHandlingMsg = false
Expand Down
79 changes: 59 additions & 20 deletions test/test_typed.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,27 +1,32 @@
@testset "Message dispatcher" begin

function socket_path(id)
if Sys.iswindows()
global_socket_name1 = "\\\\.\\pipe\\jsonrpc-testrun1"
return "\\\\.\\pipe\\jsonrpc-testrun$id"
elseif Sys.isunix()
global_socket_name1 = joinpath(tempdir(), "jsonrpc-testrun1")
return joinpath(mktempdir(), "jsonrpc-testrun$id")
else
error("Unknown operating system.")
end
end

@testset "Message dispatcher" begin

global_socket_name1 = socket_path(1)

request1_type = JSONRPC.RequestType("request1", Foo, String)
request2_type = JSONRPC.RequestType("request2", Nothing, String)
notify1_type = JSONRPC.NotificationType("notify1", String)

global g_var = ""

server_is_up = Base.Condition()
server_is_up1 = Base.Condition()

server_task = @async try
server = listen(global_socket_name1)
notify(server_is_up)
notify(server_is_up1)
yield() # don't want to deadlock
sock = accept(server)
global conn = JSONRPC.JSONRPCEndpoint(sock, sock)
global msg_dispatcher = JSONRPC.MsgDispatcher()
msg_dispatcher = JSONRPC.MsgDispatcher()

msg_dispatcher[request1_type] = (conn, params) -> begin
@test JSONRPC.is_currently_handling_msg(msg_dispatcher)
Expand All @@ -39,7 +44,7 @@
Base.display_error(stderr, err, catch_backtrace())
end

wait(server_is_up)
wait(server_is_up1)

sock2 = connect(global_socket_name1)
conn2 = JSONRPCEndpoint(sock2, sock2)
Expand All @@ -63,22 +68,17 @@

# Now we test a faulty server

if Sys.iswindows()
global_socket_name2 = "\\\\.\\pipe\\jsonrpc-testrun2"
elseif Sys.isunix()
global_socket_name2 = joinpath(tempdir(), "jsonrpc-testrun2")
else
error("Unknown operating system.")
end
global_socket_name2 = socket_path(2)

server_is_up = Base.Condition()
server_is_up2 = Base.Condition()

server_task2 = @async try
server = listen(global_socket_name2)
notify(server_is_up)
notify(server_is_up2)
yield() # don't want to deadlock
sock = accept(server)
global conn = JSONRPC.JSONRPCEndpoint(sock, sock)
global msg_dispatcher = JSONRPC.MsgDispatcher()
msg_dispatcher = JSONRPC.MsgDispatcher()

msg_dispatcher[request2_type] = (conn, params)->34 # The request type requires a `String` return, so this tests whether we get an error.

Expand All @@ -91,7 +91,7 @@
Base.display_error(stderr, err, catch_backtrace())
end

wait(server_is_up)
wait(server_is_up2)

sock2 = connect(global_socket_name2)
conn2 = JSONRPCEndpoint(sock2, sock2)
Expand All @@ -104,6 +104,45 @@
close(sock2)
close(conn)

fetch(server_task)
fetch(server_task2)


# Now we test a wrongly requested method

global_socket_name3 = socket_path(3)

server_is_up3 = Base.Condition()

server_task3 = @async try
server = listen(global_socket_name3)
notify(server_is_up3)
yield() # don't want to deadlock
sock = accept(server)
global conn = JSONRPC.JSONRPCEndpoint(sock, sock)
msg_dispatcher = JSONRPC.MsgDispatcher()

run(conn)

for msg in conn
@test_throws ErrorException("Unknown method 'request2'.") JSONRPC.dispatch_msg(conn, msg_dispatcher, msg)
flush(conn)
end
catch err
Base.display_error(stderr, err, catch_backtrace())
end

wait(server_is_up3)

sock3 = connect(global_socket_name3)
conn3 = JSONRPCEndpoint(sock3, sock3)

run(conn3)

@test_throws JSONRPC.JSONRPCError(-32601, "Unknown method 'request2'.", nothing) JSONRPC.send(conn3, request2_type, nothing)

close(conn3)
close(sock3)
close(conn)
fetch(server_task3)

end