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

Unify return_headers option #291

Open
beligante opened this issue Jan 10, 2023 · 0 comments
Open

Unify return_headers option #291

beligante opened this issue Jan 10, 2023 · 0 comments

Comments

@beligante
Copy link
Contributor

Is your feature request related to a problem? Please describe.
The library offers the option to receive the headers from the http request. Essentially when you pass the option - in the right place - return_headers: true the library will return the headers and the trailers (those are the headers returned at the end of the request)

So, for a unary rpc you can receive the headers using:

iex(22)> alias Routeguide.RouteGuide.Stub
iex(23)> alias Routeguide.Point

iex(25)> opts = [interceptors: [GRPC.Client.Interceptors.Logger]]
iex(26)> {:ok, channel} = GRPC.Stub.connect("localhost:10000", opts)

iex(28)> Stub.get_feature(channel, Point.new(latitude: 409_146_138, longitude: -746_188_906), return_headers: true) |> IO.inspect()
{:ok,
 <name: "Berkshire Valley Management Area Trail, Jefferson, NJ, USA", location: <latitude: 409146138, longitude: -746188906>>,
 %{
   headers: %{
     "content-type" => "application/grpc+proto",
     "date" => "Tue, 10 Jan 2023 12:28:19 GMT",
     "server" => "Cowboy"
   },
   trailers: %{"grpc-message" => "", "grpc-status" => "0"}
 }}

And in the case of a bidirectional stream we need to pass that option in the GRPC.Stub.recv/2

iex(39)> stream = channel |> Routeguide.RouteGuide.Stub.route_chat()
iex(40)> point = Routeguide.Point.new(latitude: 0, longitude: 1)
iex(41)> note = Routeguide.RouteNote.new(location: point, message: "message")
iex(42)> GRPC.Stub.send_request(stream, note, end_stream: true)
iex(43)> {:ok, ex_stream, %{headers: headers}} = GRPC.Stub.recv(stream, return_headers: true)
{:ok, #Function<61.127921642/2 in Stream.unfold/2>,
 %{
   headers: %{
     "content-type" => "application/grpc+proto",
     "date" => "Tue, 10 Jan 2023 12:37:57 GMT",
     "server" => "Cowboy"
   }
 }}
iex(44)> Enum.to_list(ex_stream) |> IO.inspect
[
  ok: %Routeguide.RouteNote{
    __unknown_fields__: [],
    location: <latitude: 0, longitude: 1>,
    message: "message"
  },
  ok: %Routeguide.RouteNote{
    __unknown_fields__: [],
    location: <latitude: 0, longitude: 1>,
    message: "message"
  },
  ok: %Routeguide.RouteNote{
    __unknown_fields__: [],
    location: <latitude: 0, longitude: 1>,
    message: "message"
  },
  trailers: %{"grpc-message" => "", "grpc-status" => "0"}
]

Notice the following for the above:

  1. I had to pass the option in another function ( recv/2 )
  2. The headers were returned in a three position tuple and the trailers were returned inside the Elixir.Stream

Describe the solution you'd like

For this issue there are two problems that I would like to solve.

1

I would like to unify the place where we pass that option and my suggestion here is to pass that when we're starting the request. In this case, unary call stay unchanged. But for bidi-streams we could pass the option like this:

stream = Routeguide.RouteGuide.Stub.route_chat(channel, return_headers: true)
# no longer passing the option in recv/2
{:ok, ex_stream, %{headers: headers}} = GRPC.Stub.recv(stream)

2

For bidi-streams we could return the headers as a member of the Elixir.Stream. In this case, when we invoke recv/2 we would return a two position tuple. Like:

stream = Routeguide.RouteGuide.Stub.route_chat(channel, return_headers: true)
# no longer receiving a three position tuple in recv/2
{:ok, ex_stream} = GRPC.Stub.recv(stream)
iex(17)> [ex_stream |> Enum.to_list() |> IO.inspect()
[
  headers: %{
    "content-type" => "application/grpc+proto",
    "date" => "Tue, 10 Jan 2023 12:49:47 GMT",
    "server" => "Cowboy"
  },
  ok: %Routeguide.RouteNote{
    __unknown_fields__: [],
    location: <latitude: 0, longitude: 1>,
    message: "message"
  },
  trailers: %{"grpc-message" => "", "grpc-status" => "0"}
]

Additional context
This will be a breaking change and also will require changes on both (gun and min) adapters

@beligante beligante changed the title Unify receive_headers option Unify return_headers option Jan 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant