keathley/twirp-elixir

Elixir implementation of the twirp RPC framework

twirp-elixir's Issues

Implement Twirp protocol v7

Changes required to implement the new protocol:

• Error code resource_exhausted should map to 429 (instead of 403).
• Allow any arbitrary URL prefixes instead of forcing the "/twirp".

See Protocol v7 differences and the Go implementation v7.0.0 release notes for details.

Read comments in protobuf and add them as docs to client functions

Support for `oneof` attributes missing when using JSON?

Thank you for this package!

Assume the following protobuf definition:

 message Result {
  oneof result {
    Success success = 1;    
    Error error = 2;          
  }

  message Success {
    string processed_at = 1;  
  }

  message Error {
    int32  status = 1;        
    string title = 2;         
    string detail = 3;        
  }
}

The elixir struct looks this:

defmodule Result do
  @moduledoc false
  use Protobuf, syntax: :proto3

  @type t :: %__MODULE__{
          result:
            {:success, Result.Success.t() | nil}
            | {:error, Result.Error.t() | nil}
        }

  defstruct result: nil

  oneof :result, 0

  field :success, 1, type: Result.Success, oneof: 0
  field :error, 2, type: Result.Error, oneof: 0
end

If I create new Result:

    success = Result.Success.new(processed_at: processed_at)
    Result.new(result: {:success, success})

An error is returned from the RPC if I use JSON as output format:

protocol Jason.Encoder not implemented for {:success,....

But if I use the Protobuf.JSON.to_encodable/1 from :protobuf it correct (lowerCamelCase):

%{"success" => %{"processedAt" => ...}}

Solution: Instead of using Jason to convert the generated structs, the function Protobuf.JSON.to_encodable/1 should used to convert the struct in a previous step.

Of course, it is possible to use the Twirp.Error stuff, but this is only an example, that in case of oneof definition the Jason encoder should not called direct on the structs, because the oneof attribute uses tuples like {:key, value} to support several options.

(KeyError) key :elixir_module_prefix not found in: %Google.Protobuf.FileOptions{}

(KeyError) key :elixir_module_prefix not found in: %Google.Protobuf.FileOptions{}

https://github.com/keathley/twirp-elixir/blob/master/lib/twirp/protoc/generator.ex#L25 it appears protobuf-elixir/protobuf no longer has that key though it looks like it might exist as some sort of monkey-patched protobuf extension if you require the right module.

I'm sorry this isn't a patch, I'm new to Elixir and unsure if we should be testing for the key or requiring in the extension.

Publishing clients to hex

We want to be able publish generated clients to our private hex registry. This presents some interesting challenges.

Ideally, we'd be able to publish the client directly from the service that defines it. But this isn't quite possible because the client has a dependence on twirp and the HTTP client. We can't publish the entire service mix.exs file since it would include all of the dependencies that the service is using and there might be conflicts between the provider service and the consumer service.

I'm not sure that there is a great way around this. The only thing I can think of is to create a separate mix project that contains the service definition and the client. Both the providing service and the consuming service would need to import it as a dependency. That seems annoying. But I'm not sure I have a better solution.

Client generation should have no dependency on twirp itself

This caused an issue a while ago where the generated client depended on a more recent version of twirp. Based on experience, I think we want to generate the client in such a way as to have no dependency other than the http client.

telemetry version conflicts in newer app

Getting errors importing twirp-elixir into applications that are using telemetry version 1.

Looking at how other libraries are handling this, it appears that supporting both .4 and 1.0 is a common pattern.
https://github.com/phoenixframework/phoenix/blob/master/mix.exs#L66
https://github.com/elixir-ecto/ecto/blob/master/mix.exs#L35

#119 -- follows the above approach