search

dialohq / ocaml-grpc

gRPC library for OCaml
https://dialohq.github.io/ocaml-grpc
BSD 3-Clause "New" or "Revised" License
62 stars 9 forks source link

ocaml-grpc does not work with Clickhouse gRPC interface #38

Closed vbmithr closed 1 year ago

vbmithr commented 1 year ago

Original bug report at: https://github.com/ClickHouse/ClickHouse/issues/52465 Could not figure out why it is not working just by looking at Wireguard traces, it seems related to an HTTP2 behaviour that their server implem does not like… Just posting this here for reference, but I would be happy if anybody has an answer to this :)

quernd commented 1 year ago

Hi, thanks for reporting this! Unfortunately I can't download your Wireguard traces (403 Forbidden), could you double-check please? Also, do you have a short reproduction in code? I have access to a Clickhouse deployment I could try to reproduce against.

vbmithr commented 1 year ago

Link restored, sorry about this. And will try to post a reproduction code too.

quernd commented 1 year ago

Thank you, that's very helpful actually.

I compared the two requests and I noticed that the successful one (using the Python implementation) sends two headers that the OCaml request doesn't send. One of them is user-agent (I don't think that's an issue), but the other one is :authority and I do remember having had that same problem (server giving me INTERNAL_ERROR (2)) with some API a while ago.

Could you try sending this header and see if it solves the issue? Something along these lines:

let headers =
  H2.Headers.of_list
    [
      (":authority", "127.0.0.1:9100");
      ("te", "trailers");
      ("content-type", "application/grpc");
    ]

and then you supply the headers in your gRPC request, like Client.call ~headers.

If that works, we should probably put a warning somewhere about this, or make it harder to omit this header (optional argument so it's at least an explicit part of the API?). I'm not sure about the best way to handle this, since according to the gRPC over HTTP2 spec the :authority pseudo-header is optional.

vbmithr commented 1 year ago

Okay, amazing, that was it!

tmcgilchrist commented 1 year ago

@vbmithr I've been using the Go implementation https://github.com/grpc/grpc-go/ as my defacto source of truth to go along with the spec. It does seem to be optional there too. Can we mark this as solved if that is the case?

vbmithr commented 1 year ago

Yeah, sure. It's annoying that some servers request it and silently fail when not set, though :/ But I agree that there is not much that can be done about it here. Thanks.