nats-rpc/nrpc

Improve the response (rebooted)

cdevienne opened this issue · 2 comments

We recently merged #13, which support fully explicit responses. It has the big advantage of avoiding a double encoding of the response, and in case of replies with simple types, the handler and wrapper signatures gets even more simplified.

After concretely using it I can however underline a few drawbacks:

  • The use of "oneof" generates structures that are tedious to deal with
  • The use of "oneof" introduces ambiguities in the json format. Using gogo/protobuf even generates tests of json encoding/decoding that fails(!)
  • Explicitly defining the replies is boring, and does not improve the readability of the .proto files. It also makes the experience significantly different from grpc.

These drawbacks are not enough for me to switch back to the former system, however I think a 3rd way may solve all the problems.

We want to:

  • avoid using "oneof"
  • avoid a double encoding
  • avoid fully explicit definitions

I propose to use a custom encoding for the response:

  • A successful reply is the normal protobuf encoding of the output type
  • A error reply is a "0" byte followed by a protobuf encoding of a nrpc.Error

With this approach, we have an easy, cheap and unmistakable way to detect if the response is an error (test the first byte of the buffer), at the cost of an extra byte only in case of error.
Also, we should get a slightly smaller payload, as a successful response would only encode the output type and send it unwrapped.

As for the json encoding, since we get to define the wire format, and replies are necessarily json objects, we could define that an error reply would have the following structure:

{ "__error__": { .... }}

This could be tested even before actually decoding the json, by probing the first 15 first bytes of so, which remains cheap.

Thoughts ?

Another argument in favor is that an error reply can always be sent to the client when the subject is invalid.
It implies removing the fully-specified replies support (which is probably fine, as it also simplify the code of nrpc and the generated code).

Fixed by #27