WebAssembly / wasi-http

Other
153 stars 24 forks source link

Additions to support gRPC #128

Open dicej opened 2 months ago

dicej commented 2 months ago

Recently, I've been trying to use a popular .NET library, Grpc.Net, to make outbound gRPC connections using a port of System.Net.Http to wasi-http. I was able to get it to work, but had to hack a few things:

Ideally, none of those hacks would be necessary, so I'd like to propose the following additions to the wasi-http spec:

See also https://github.com/bytecodealliance/wasmtime/pull/7538 for further discussion.

tschneidereit commented 2 months ago

I agree that we should unblock gRPC over wasi-http. For the specific way of going about it, I wonder if we might be able to get away with something slightly less invasive:

The combination of these two would I think fully satisfy the requirements for making gRPC work, while not requiring any additions to the WIT interfaces of wasi-http. (And technically not even any spec changes, given that the list of forbidden headers isn't currently part of the spec itself.)

At first glance, this suggestion (or the changes Joel suggests above) seem bad in that they aren't supportable in browser environments, because TE is a forbidden header in browsers. This seems particularly bad because it's not statically visible that a browser environment will not be able to run a given component.

However, I'd argue that if anything allowing TE: trailers is an improvement over the status quo: it's already the case that trailers themselves aren't supported in browsers, so trying to send them will inevitably fail. Given that, it seems strictly better to have an error at a time when it might still be actionable: before even sending a request, instead of after the request has been sent in its entirety, except for the trailers.

dicej commented 2 months ago

@tschneidereit In your proposal how would the implementation indicate that, although the client sent an HTTP/2 request, the server downgraded the response to e.g. HTTP/1.1?

tschneidereit commented 2 months ago

You're right, I didn't address that part. One possible answer would be that we'd expose that as a connection error, because it'd mean that there's no reliable way to support the required semantics.

In practice, I would assume that someone who wants to use trailers (and hence sends TE: trailers) would do so in an environment that will successfully transport the trailers—meaning the host the request is sent to speaks http/2+. So an error would rarely happen in reality, and would probably be the right result if it does.

lukewagner commented 2 months ago

Yes, also agreed on the goal of allowing gRPC to work over wasi-http. Riffing on Till's idea, could we add a method roughly like:

resource outgoing-request {
  ...
  expects-trailers: func() -> bool;
  set-expects-trailers: func(required: bool);
}

where setting expects-trailers causes the host to set TE: trailers as appropriate and also causes the handle call to fail with a connection error if the host determines that the transport doesn't support trailers (again giving the host some lattitude to judge the HTTP/1.1 situation)?

tschneidereit commented 2 months ago

That approach would work, and I agree that it'd be conceptually cleaner. It'd also come at significant cost to portability: it'd mean that any code that would "just work" under my proposal would instead have to be changed. That'll either affect code that's easy to change, but needs changing in many places (potentially multiple per application, with the risk of not catching all of them) or in hard-to-change ones, such as deep within frameworks/libraries that are slow to update, and often not under the interested party's control.

Given that, I do wonder if the conceptual cleanliness is really worth it?

tschneidereit commented 2 months ago

As a quick heads-up, Joel and I talked about this in much more detail and realized that neither of the last two suggestions will really work. We did come up with an alternative that seems much better, which I'll sketch out here later today

tschneidereit commented 2 months ago

I didn't get around to this before EOD yesterday, but I now opened #129 with proposed API additions to address the requirements here.

I wrote a long explanation of my reasoning behind this design in the PR, so won't repeat that here :)

pavelsavara commented 2 weeks ago

The design https://github.com/WebAssembly/wasi-http/pull/129 proposes new interface per HTTP version. But there are more dimensions to the HTTP versioning and features. In the discussion on that PR we concluded that for now, we need to go ahead with more dynamic options on the current (generic HTTP) interface.

Joel's hack of wasmtime header validation https://github.com/bytecodealliance/wasmtime/compare/main...dicej:wasmtime:grpc-hacks is just removing the header validation.

Do I understand correctly that next steps here are

Am I missing some steps ?

lukewagner commented 1 week ago

One recent idea that came up from an in-person discussion with @tschneidereit was that, similar to how we've added special-case handling for Content-Length in the spec, we could also add special-case handling for TE that allows TE: trailers and specifies how it behaves. The last I heard, it sounded like this might be sufficient and subsume the version stuff (which, all else being equal, we'd like to abstract over). @tschneidereit or others: am I remembering correctly and/or is there any newer thinking on this approach?