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.

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

Missing error handling case in dispatch_msg #90

Open
BambOoxX opened this issue Jan 21, 2025 · 0 comments · May be fixed by #91
Open

Missing error handling case in dispatch_msg #90

BambOoxX opened this issue Jan 21, 2025 · 0 comments · May be fixed by #91

Comments

@BambOoxX
Copy link

Hi, while testing some functionnalities, I met an error that was not handled by dispatch_msg.
Indeed, if there is a test inside the constructor of some parameter of the call, like an @assert this errors, and breaks the JSONRPC communication because it does not return a JSONRPCError that could be properly handler later on.
The implementation below catches any error occurring during that step.
Do you think this could be a relevant addition ?

function JSONRPC.dispatch_msg(x::JSONRPCEndpoint, dispatcher::MsgDispatcher, msg::Request)
    dispatcher._currentlyHandlingMsg = true
    try
        method_name = msg.method
        handler = get(dispatcher._handlers, method_name, nothing)
        if handler !== nothing
            param_type = get_param_type(handler.message_type)
            params = try
                param_type === Nothing ? nothing : param_type <: NamedTuple ? convert(param_type, (; (Symbol(i[1]) => i[2] for i in msg.params)...)) : param_type(msg.params)
            catch ex
                # Return an error if the conversion to the requested type fails for some reason
                JSONRPCError(-999999, "Conversion error", nothing)
            end

            if handler.message_type isa RequestType
                handler.func(x, params, msg.token)
            else
                handler.func(x, params)
            end
            if handler.message_type isa RequestType
                if res isa JSONRPCError
                    send_error_response(x, msg, res.code, res.msg, res.data)
                elseif res isa get_return_type(handler.message_type)
                    send_success_response(x, msg, res)
                else
                    error_msg = "The handler for the '$method_name' request returned a value of type $(typeof(res)), which is not a valid return type according to the request definition."
                    send_error_response(x, msg, -32603, error_msg, nothing)
                    error(error_msg)
                end
            end
        else
            error("Unknown method $method_name.")
        end
    finally
        dispatcher._currentlyHandlingMsg = false
    end
end
@BambOoxX BambOoxX changed the title Missing error handling case in Missing error handling case in dispatch_msg Jan 21, 2025
@davidanthoff davidanthoff linked a pull request Jan 22, 2025 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant