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

Is that possible for custom error to pop up as rpc::error in json-rpc? #912

Open
ralphjzhang opened this issue Apr 12, 2024 · 4 comments · May be fixed by #913
Open

Is that possible for custom error to pop up as rpc::error in json-rpc? #912

ralphjzhang opened this issue Apr 12, 2024 · 4 comments · May be fixed by #913
Labels
bug Something isn't working

Comments

@ralphjzhang
Copy link

Try to return custom errors from json-rpc server methods like

struct MyParams {};
struct MyResult {};
using MyServer = rpc::server<
    rpc::method<"MyMethod", MyParams, std::expected<MyResult, rpc::error>>>;
using MyClient = rpc::client<
    rpc::method<"MyMethod", MyParams, std::expected<MyResult, rpc::error>>>;

int main() {
  MyServer server;
  server.on<"MyMethod">([](MyParams) { return rpc::error{}; });

  MyClient client;
  auto [req, _] =
      client.request<"MyMethod">(1, MyParams{}, [](auto val, auto id) {});
  auto resp = server.call(req);

  std::println("resp: {}", resp);
}

but the response actually use the returned error as the result payload, i.e. the result look like

resp: {"jsonrpc":"2.0","result":{"unexpected":{"code":0,"message":"No error"}},"id":1}

instead of using the error as json-rpc error, like

resp: {"jsonrpc":"2.0","error":{"code":0,"message":"No error"},"id":1}

Is that possible to integrate error handling with the method handler?

@ralphjzhang
Copy link
Author

I realize that adding some helper:

   template <typename Result>
   struct expected_helper
   {
      static constexpr bool is_expecting_rpc_error = false;
      using expected_t = std::expected<Result, rpc::error>;
   };
   template <typename Expected>
   struct expected_helper<expected<Expected, rpc::error>>
   {
      static constexpr bool is_expecting_rpc_error = true;
      using expected_t = std::expected<Expected, rpc::error>;
   };

and use it to extract those methods returning expected<T, rpc::error> would do the job, would that PR get accepted?

@stephenberry
Copy link
Owner

Ah, that was an unintended consequence of adding general support for std::expected. This should definitely be fixed. I'll experiment with a solution before I ask for a PR. Thanks for bringing this up.

@stephenberry stephenberry added the bug Something isn't working label Apr 12, 2024
@stephenberry stephenberry linked a pull request Apr 12, 2024 that will close this issue
@stephenberry
Copy link
Owner

After thinking about this some more, I have some questions. Are you trying to replace rpc::error with your own error class?

@ralphjzhang
Copy link
Author

So far I find using rpc error is fine for me, but from design point of view, maybe it's a good idea to allow using, e.g, derived class of rpc error or some sort ...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants