An attempt at trailers

[itowlson]

I was trying to get a gRPC sample to work in Spin, but it choked on a couple of things:

• It wanted the request to be HTTP/2 (fixable in the handler by overriding ProtoMajor)
• It wanted the ResponseWriter to be Flusher (not fixable in the handler, easy fix in SDK)
• It wanted some gRPC trailers (not fixable in the handler because the SDK has a TODO on trailers)

I don't understand the trailers stuff well. I'm not very confident I'm doing it at the right time or in the right way. And I think we might also need to adopt a Go kludge where it allows non-predeclared trailers to be recorded after the headers have been written by naming them Trailer:Foo - my simple gRPC test so far hasn't turned up a need for that, but the net.http source suggests they did it because gRPC needed it (https://cs.opensource.google/go/go/+/master:src/net/http/internal/http2/server.go;l=2674?q=responsewriter&ss=go%2Fgo).

So please be gentle with me, this is a pretty naive effort to get one minimal thing working, and I am putting it up not as a polished offering but so other folks can provide guidance on whether the direction is roughly right. Thanks!

(Also please forgive any stupid naming, there was a lot of flailing on the way to this point and I might not have backed it all out!)

[itowlson]

Now tested with all four gRPC communication patterns (using the Google route guide sample and grpcurl), and this seems to suffice. So I'm marking this ready for review but again I realise it will probably need some love from Go and WASI folks first!

[itowlson]

I added the gRPC sample in because that's what motivated this (in a separate commit so we can back it out if we don't want it in this PR). @adamreese do you have a better solution for persuading gRPC that the request is HTTP/2? Mine is a ghastly kludge

[dicej]

LGTM; just a couple of suggestions inline. Thanks for doing this!

[adamreese]

Thanks so much for this. The example is awesome! Sorry for the large suggestion diff, this felt more readable than a bunch of nitty comments. I didn't read the code in the example too closely yet but I'll give it a pass next.

[adamreese]

This should return any errors

[itowlson]

This is called from a defer (https://github.com/itowlson/spin-go-sdk/blob/traileroo/http/http.go#L59) - what should I do with any errors?

[adamreese]

This one is going to be a challenge. The defer we can hack around but the errors are being sent through a channel. The issue being that we've already read that channel for the response. We're going to run into deadlocks. We might need to log it. Still thinking...

[itowlson]

I'm an idiot. The trailers future is a Result. So we can pass any conversion error via that future as a internal error code. Please take a look (I put it in a separate commit).

I also moved the writeTrailers call out of the defer and into the main flow, which feels safer and more explicit, and gives us a better venue for error handling if we need it. The gRPC sample still works so fingers crossed this doesn't have some weird side-effect!

[adamreese]

Good catch! That's way cleaner than the road I was going down.

[adamreese]

filePath is always an empty string. Is this function to show how you could load a file or should we simplifiy?

[itowlson]

It was to minimise changes to the Google code. I wanted to keep the bulk as was (so readers familiar with the gRPC sample would see that it Just Worked), and only change how we entered it. Maybe that plan was more confusing than changing it...

[itowlson]

@adamreese I updated the comments to clarify, and added some context in the README and inline comments pointing back to the Google sample (and its licence).

[adamreese]

Thanks again for this @itowlson!