jsonrpcx / json-rpc-cxx

JSON-RPC for modern C++
MIT License
251 stars 44 forks source link

Incorrect error message when an nlohmann type_error is thrown in rpc method #48

Open samkottler opened 1 month ago

samkottler commented 1 month ago

If there is an unhandled type_error thrown in the rpc method itself, the error should be an internal error not an invalid parameters error. For example, this program

#include "nlohmann/json.hpp"
#include "jsonrpccxx/server.hpp"
#include <iostream>

std::string do_something()
{
  nlohmann::json json = 0;
  std::string my_string = json;
  return my_string;
}

int main()
{
  jsonrpccxx::JsonRpc2Server handler;
  handler.Add("do_something", jsonrpccxx::GetHandle(&do_something), {});

  std::string request = R"({"jsonrpc": "2.0", "id": 0, "method": "do_something", "params": []})";

  std::cout << handler.HandleRequest(request) << std::endl;
}

prints

{"error":{"code":-32602,"message":"invalid parameter: [json.exception.type_error.302] type must be string, but is number"},"id":0,"jsonrpc":"2.0"}

which doesn't make sense because the exception is thrown in the body of do_something, not when converting parameters.

cinemast commented 1 month ago

Do you have a reference to the spec that supports this idea?

In my opinion if there is something wrong with parameters, it is fine to return this error

samkottler commented 1 month ago

I agree that if something is wrong with the parameters, this is the correct response. However, my example shows the opposite problem. It returns the invalid parameters error even though the actual error has nothing to do with the parameters.

cinemast commented 1 month ago

Sorry, I shouldn't answer in the middle of the night. You are totally right about this. The issue is here I guess: https://github.com/jsonrpcx/json-rpc-cxx/blob/master/include/jsonrpccxx/dispatcher.hpp#L59

We need to catch errors coming from functions earlier before we fall into this catch block.