New grpcadapter package to transcode connect and gRPC-Web #527

emcfarlane · 2023-06-21T13:02:48Z

Creates a new package grpcadapter (github.com/connect-go/grpcadapter). Create a handler grpcadapter.NewHandler that can transcode connect and gRPC-web protocols to gRPC handlers. Allows gRPC go servers to use connect by wrapping grpc.Server.

	grpcServer := grpc.NewServer()
	mux := grpcadapter.NewHandler(grpcServer,
		grpcadapter.WithReadMaxBuffer(1024*1024),
		grpcadapter.WithWriteMaxBuffer(1024*1024),
	)
	http.ListenAndServe(":8080", h2c.NewHandler(mux, &http2.Server{}))

Unary connect requests are buffered and obey the limits set via connect options. For streams and gRPC-web these options are ignored and control is passed to the underlying gRPC handler.

Connect-crosstest is used to add more coverage: connectrpc/conformance#478

Closes https://github.com/bufbuild/connect-go/issues/334

* Add test for errors marshaling protobuf Status If a user-supplied Codec errors when marshaling a protobuf Status message (which we use internally when converting errors to HTTP trailers), we currently drop all the error information on the floor and return "Unknown: EOF". This commit adds a reproduction as a test case. Relates to #197. * Use CodeInternal for errors marshaling proto Status If we can't marshal a protobuf Status message, send CodeInternal to the client with some details. * Fix error-wrapping lint

Fixes #205.

So that we can share code between our implementations of the Connect and gRPC protocols, factor enveloping (aka length-prefixed messages) and compression out of the gRPC-specific code.

* Add test for unary RPC with zero-byte messages * Add failing test for handler timeout handling We're parsing timeouts, but not properly propagating them into user code. Thanks, crosstests! * Fix timeout handling Since we know the shape of the Connect protocol, we can simplify the protocol interfaces and move some shared utility functions into `protocol.go`. This also fixes server-side timeout handling. * Keep Accept-Post string manipulation shorter We're only doing this at startup, so it's okay to make it slow. The code doesn't get much shorter, but it's arguably more readable. * Add indirection to constant limit Move the literal for our discard limit into a constant.

…nable to be sent (#211) Co-authored-by: Steve Ayers <sayers@uber.com>

…est is unable to be sent (#211)" This reverts commit eaacdf5.

In preparation for factoring out some of the complicated HTTP stuff happening in the gRPC client stream implementation, move timeout encoding to a less-convoluted portion of the code.

The most complex portion of the client-side gRPC sender and receiver is the HTTP layer, where we use `io.Pipe` to create a streaming request body. So that we can reuse this code for the Connect protocol, factor it out of the gRPC-specific code. This has the side effect of simplifying the gRPC implementation and more clearly separating the gRPC and gRPC-Web protocols.

The params structs for protocols are somewhat proven now, so we can stop hand-copying them into protocol-specific structs and embed instead.

Make all the gRPC headers constants and prefix a few unexported utility functions that snuck through.

If we're not using the receiver, leave it unnamed.

Use Connect's canonical string representation for error codes.

We use sender and receiver wrappers to make sure that all public APIs return *Errors. Factor these out of the gRPC code, since they're equally applicable to Connect.

emcfarlane · 2023-06-26T18:11:27Z

grpcadapter/transport.go

+// binary Protobuf format, even if the messages in the request/response stream
+// use a different codec. Consequently, this function needs a Protobuf codec to
+// unmarshal error information in the headers.
+func grpcErrorFromTrailer(trailer http.Header) *connect.Error {


Copied from protocol_grpc.go

emcfarlane · 2023-06-26T18:11:44Z

grpcadapter/transport.go

+	return base64.NewDecoder(base64.URLEncoding, stringReader)
+}
+
+func queryValueReader(data string, base64Encoded bool) io.Reader {


Copied from protocol_connect.go.

Use the `MarshalAppend` methods introduced in v1.31 of the protobuf runtime to more aggressively reuse buffers from Connect's internal pool (for both binary and JSON). This reduces allocations and substantially improves throughput: ``` BenchmarkConnect/unary-12 8054 1318347 ns/op 14414394 B/op 251 allocs/op BenchmarkConnect/unary-12 10448 1153636 ns/op 6115680 B/op 236 allocs/op ``` --------- Co-authored-by: Akshay Shah <akshay@akshayshah.org>

Capture unexpected io.EOF errors as io.ErrUnexpectedEOF in client streams. Now raises an error when the stream doesn't end with an end response message. Fixes https://github.com/bufbuild/connect-go/issues/397

akshayjshah

I really like the direction here - the user-facing API is super nice and clean! It's also much more decoupled from the internals of connect-go than I expected, which is great - I'd love to keep this PR open until we've got github.com/connectrpc ready to go, and then put this code into a separate repository.

A few notes on the code:

Please follow the Go style guide. (Search Notion for "Code Standards.")
Can we use the content-length header to buffer less when handling unary Connect RPCs?

connect_ext_test.go

grpcadapter/adapter.go

connect_ext_test.go

grpcadapter/adapter.go

protocol.go

protocol_connect.go

Cover the gRPC and gRPC-Web protocols in the tests for `io.ErrUnexpectedEOF`, and ensure that error codes and observable `errors.Is` behavior is consistent for all 3 supported RPC protocols.

timruffles · 2023-08-17T11:00:13Z

What's the status of this feature, and any indication on which version it'll appear in?

akshayjshah · 2023-08-17T16:46:49Z

We're splitting it out into a separate project, which we'll release under the https://github.com/connectrpc organization. It's being actively worked on by @emcfarlane and @jhump, so it's coming as soon as we get something really nice ready!

timruffles · 2023-09-05T15:40:27Z

That's great. Do you know if it will support existing unary server interceptors? We have a lot of existing interceptors we'd like to avoid having to rewrite or maintain 2x copies of.

akshayjshah · 2023-09-05T16:25:58Z

As things are shaping up now, the new package we're developing works with any http.Handler. Since grpc.Server implements http.Handler, you'll have full support for unary and streaming gRPC interceptors.

timruffles · 2023-09-20T16:45:15Z

Is there somewhere we could follow along the development of the forthcoming package? We're getting close to an internal decision point about technology, and having this available would really help. The number of internal existing golang gRPC services we have implemented means it wouldn't make sense to use connect without something like this.

timruffles · 2023-09-20T16:52:00Z

Also - we'd be very happy to beta-test! 🥳

emcfarlane · 2023-09-21T13:32:23Z

Hey @timruffles, beta testing would be great! Will get something to you soon.

bufdev and others added 30 commits April 9, 2022 11:08

Pool buffers per handler and client (#192)

b574f42

Mark repository as a work in progress

cfcee6f

Improve error when clients try bidi over HTTP/1.x (#203)

4151ebe

Fix Make dependency (#202)

08e995b

Do not return error from Client constructor (#204)

dea5933

Cleanup (#206)

8a73466

Update names in CONTRIBUTING.md (#207)

fd9ace4

Rename Specification -> Spec (#208)

9161c3d

Fixes #205.

Factor enveloping and compression out of gRPC (#209)

c35ee25

So that we can share code between our implementations of the Connect and gRPC protocols, factor enveloping (aka length-prefixed messages) and compression out of the gRPC-specific code.

Returns a more intuitive Connect error if the underlying request is u…

eaacdf5

…nable to be sent (#211) Co-authored-by: Steve Ayers <sayers@uber.com>

Revert "Returns a more intuitive Connect error if the underlying requ…

532e99d

…est is unable to be sent (#211)" This reverts commit eaacdf5.

Move gRPC timeout encoding into NewStream

bba59af

In preparation for factoring out some of the complicated HTTP stuff happening in the gRPC client stream implementation, move timeout encoding to a less-convoluted portion of the code.

Improve error message

5abed41

Embed instead of copying

b35b2ca

The params structs for protocols are somewhat proven now, so we can stop hand-copying them into protocol-specific structs and embed instead.

Rename & move gRPC-specific HTTP mappings

8f935d0

Rename & move gRPC-specific error unmarshaling

4334b8c

Rename & move gRPC handler and marshaler

729b6c2

Rename & move gRPC-specific timeout code

27d90b0

Rename & move gRPC utilities

b187dcd

Move header keys to constants, prefix utilities

3eb41b2

Make all the gRPC headers constants and prefix a few unexported utility functions that snuck through.

Factor request URL validation out of gRPC

f6aa67a

Clean up gRPC protocol receivers

b272a7c

If we're not using the receiver, leave it unnamed.

Change Code.String() to match Connect protocol

8c52c92

Use Connect's canonical string representation for error codes.

Factor compression negotiation out of gRPC

fc51508

Factor fallback error-coding wrappers out of gRPC

ce583d9

We use sender and receiver wrappers to make sure that all public APIs return *Errors. Factor these out of the gRPC code, since they're equally applicable to Connect.

Fix lint config for moved code

72a923a

More compact switch in protocol_grpc.go

74c75e6

emcfarlane changed the title ~~Transcode Connect and gRPC-Web to gRPC handlers~~ New grpcadapter package to transcode connect and gRPC-Web Jun 26, 2023

Create grpcadapter package

c019158

emcfarlane force-pushed the edward/transcode-handler branch from d717a91 to c019158 Compare June 26, 2023 18:06

emcfarlane commented Jun 26, 2023

View reviewed changes

emcfarlane and others added 5 commits June 27, 2023 09:22

Simplify code when pooled buffers aren't returned (#532)

daa0efc

Prepare for v1.9.0 (#534)

5e1046d

Back to development (#537)

131bbc8

Capture unexpected io.EOF errors as io.ErrUnexpectedEOF (#533)

7406ad6

Capture unexpected io.EOF errors as io.ErrUnexpectedEOF in client streams. Now raises an error when the stream doesn't end with an end response message. Fixes https://github.com/bufbuild/connect-go/issues/397

akshayjshah reviewed Jun 30, 2023

View reviewed changes

emcfarlane and others added 4 commits June 30, 2023 10:36

Surface io.EOF errors as io.ErrUnexpectedEOF for gRPC (#539)

0849a37

Cover the gRPC and gRPC-Web protocols in the tests for `io.ErrUnexpectedEOF`, and ensure that error codes and observable `errors.Is` behavior is consistent for all 3 supported RPC protocols.

Feedback fix code layout

bd0172f

Merge branch 'main' into edward/transcode-handler

574b6cb

Stream unary if Content-Length is set

06777b1

emcfarlane force-pushed the edward/transcode-handler branch from 08625c5 to 06777b1 Compare July 3, 2023 15:30

emcfarlane added 4 commits July 3, 2023 17:01

Add buffer pool

6b7c01d

Move to grpcadapt package

47028aa

Document Resource Exhausted errors

1644a53

Fix formatting

070beda

akshayjshah force-pushed the main branch from 843d045 to 3e10f8e Compare July 26, 2023 19:07

emcfarlane closed this Jul 27, 2023

akshayjshah deleted the edward/transcode-handler branch September 15, 2023 23:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New grpcadapter package to transcode connect and gRPC-Web #527

New grpcadapter package to transcode connect and gRPC-Web #527

emcfarlane commented Jun 21, 2023 •

edited

Loading

emcfarlane Jun 26, 2023

emcfarlane Jun 26, 2023

akshayjshah left a comment

timruffles commented Aug 17, 2023

akshayjshah commented Aug 17, 2023

timruffles commented Sep 5, 2023

akshayjshah commented Sep 5, 2023

timruffles commented Sep 20, 2023

timruffles commented Sep 20, 2023

emcfarlane commented Sep 21, 2023

New grpcadapter package to transcode connect and gRPC-Web #527

New grpcadapter package to transcode connect and gRPC-Web #527

Conversation

emcfarlane commented Jun 21, 2023 • edited Loading

emcfarlane Jun 26, 2023

Choose a reason for hiding this comment

emcfarlane Jun 26, 2023

Choose a reason for hiding this comment

akshayjshah left a comment

Choose a reason for hiding this comment

timruffles commented Aug 17, 2023

akshayjshah commented Aug 17, 2023

timruffles commented Sep 5, 2023

akshayjshah commented Sep 5, 2023

timruffles commented Sep 20, 2023

timruffles commented Sep 20, 2023

emcfarlane commented Sep 21, 2023

emcfarlane commented Jun 21, 2023 •

edited

Loading