connectrpc / connect-go Goto Github PK

proto.Any leaves a lot to be desired - should we change the APIs for error details? If we're confident in our ability to ship a good alternative to Any, we might want to make the Any-based APIs have slightly clunkier names and save the ergonomic names for our new type.

The interface between the generated code and the runtime shouldn't use the protobuf-centric package/service/method triple - it may not map well to future encodings (if any). Instead, we should use the procedure name (as it's used in HTTP routing) and the name to register for reflection.

We like connect, but it's a common name: it's already taken on github, PyPI, npm, and probably other non-namespaced registries, and the domain names are hard to buy.

We could compromise a bit and use connectrpc. I've squatted on the github org, https://connectrpc.org is available, and it's available on the package registries I checked.

Other options:

• bufconnect
• geerpc
• rerpc
• minrpc
• ???

#127

Follow-up to #78

Would suggest more in line with https://github.com/bufbuild/buf/blob/e026be0ea142cce87e5cba1f4f918766f5e8e8df/private/pkg/rpc/errors.go

• Godoc above each const.
• Standardize on UPPER_SNAKE_CASE for strings, i.e. String()
• Canceled vs Cancelled - choose one, likely Canceled
• Reconsider HTTP mapping. See the comments in errors.go for what maps to gRPC and Twirp, I am not sure we have an exact mapping (need to check connect's map)
• Make UnmarshalText just be the inverse of MarshalText. Right now, MarshalText marshals to a number, while UnmarshalText accepts an UPPER_SNAKE_CASE string or a number. Confusingly to me, String produces PascalCase (commented on this above). I'd recommend that UnmarshalText, MarshalText, String all deal with UPPER_SNAKE_CASE, and MarshalText basically becomes string(c.String()), unless there's some existing pattern in all other RPC frameworks I'm missing

protocolGRPC vs grpcClient, grpcHandler, etc - we probably should prefix with grpc, ie move to grpcProtocol.

Other related things:

• We should include grpc in all the grpc-related functions, ie parseTimeout, statusFromError etc
• Should we collapse the grpc-related files a bit? Not sure.
• should protocol be clientProtocol, handlerProtocol? Super nit, not important.

This variable confuses me when I read it. I think what we're trying to accomplish is to say "mount this on the service name", but by doing this via a mutable variable, I have to decode that this is what is happening when I read through the generate code.

If we're going to have this function signature for NewHandler, we should just generate the return value as a string

The function signature is also a bit wonky to me, but it may be the best of the bad options, given that it sticks directly into https://pkg.go.dev/net/http#ServeMux.Handle. I have gone with:

// PrefixedHTTPHandler is an http.Handler with a path prefix.
//
// A router should route all requests with the path prefix to this handler.
type PrefixedHTTPHandler interface {
	http.Handler
	PathPrefix() string
}

In the past, and this is effectively what we're doing with *connect.Handler, I don't know.

var _ PingServiceClient = (*pingServiceClient)(nil) // verify interface implementation

We already get this via NewPingServiceClient - this will fail if pingServiceClient doesn't implement PingServiceClient. If this assertion fails, we have a bug in our generated code.

If we're aiming for "this is just a layer on top of stdlib HTTP", I'd argue we rename procedure to path. I might also argue to standardize on /service/method instead of service/method, to make it a real path. Thoughts?

HTTP and gRPC restrict the characters allowable in headers. We should extend IsReservedHeader to validate more thoroughly (and add a corresponding check for the header value).

See grpc/grpc-go#468 for details.

The net/http (and ultimately the net.Conn) APIs don't let us set the sort of timeouts we'd like for read and write operations. At the net.Conn level, we'd like something like an idle timeout: "if no data is sent or received for N ms, close the connection". Sadly, these APIs don't exist.

We're stuck with a timeout for the whole stream (set when constructing the http.Server). Any read or write can block, potentially until the whole stream times out. This opens streaming handlers up to abuse by clients who start a stream, but then don't send any data - until the whole stream times out, they'll tie up resources on the server. We could try to work around this by letting streams access their underlying net.Conn and setting a new deadline before every read or write, but that doubles the number of syscalls and is totally gross. (Not the most technical argument, but it's true.)

We should document this carefully, so that people using streaming RPCs know what they're signing up for. They may want to keep streaming handlers on a separate http.Server with longer timeouts, they may decide to avoid them completely in public APIs, or they may decide to wrap the default DialContext function on their server to set deadlines on each read/write.

FWIW, grpc-go and the standard library both have issues tracking this exact issue: grpc/grpc-go#1229 and golang/go#16100.

I get the idea behind "don't compress if it's not worth it", but this felt automagical to me, and I might be surprised as a user debugging this - I would argue that if someone requests compression, we should compress.

I think the purpose of all of these is (relatively) similar:

handler.go:23:type handlerConfiguration struct {
client.go:22:type clientConfiguration struct {
internal/assert/assert.go:187:type optionFunc func(*params)
internal/assert/assert.go:191:type params struct {
protocol.go:47:type protocolHandlerParams struct {
protocol.go:90:type protocolClientParams struct {

I'd argue for params.

A note, I think protocolHandlerParams/protocolClientParams are relics of when grpc was a separate package, is this right (was it)? I think they likely still make sense given the number of fields though.

Also a note, I don't think assert.params actually buys us what we want - might want to limit it to the options and just pass the parameters in directly (in every function, we call newParams anyways). Super minor nit though.

Should we add support for user-written trailers? We should investigate what people are doing with grpc-go's trailer APIs before we dismiss this out of hand.

Our protoc plugin is a mess - there are huge blocks of code, with notable overlap, that we should try to DRY up.

Follow-up to #78:

Just as a rule, I'd prefer if we have Godoc for all public types unless there's a lot of pushback here. This means even things like:

// String implements fmt.Stringer.
func (c Code) String() string {
  ...
}

First-pass comments for the base package. Not very high-quality comments if I'm self-reviewing, I'd consider this a first impressions pass.

Notes for something are below a given type, not above. Also note that comments are being made as I go, so there might be stuff that makes more sense as I get along, and previous comments not relevant.

func DecodeBinaryHeader(data string) ([]byte, error)
func EncodeBinaryHeader(data []byte) string

Not crazy about having to expose these. I don't know if there's any good solution.

func NewClientFunc[Req, Res any](doer Doer, baseURL, procedure string, options ...ClientOption) (func(context.Context, *Request[Req]) (*Response[Res], error), error)
func NewClientStream(doer Doer, stype StreamType, baseURL, procedure string, options ...ClientOption) (func(context.Context) (Sender, Receiver), error)

Func vs Stream to denote unary vs streaming here feels a bit off to me, can't put a finger on it. NewClientUnaryFunc, NewClientStreamFunc feels too long. I don't know the right answer. I do think that standardizing naming for unary vs streaming across the whole API would be good though.

It also feels like StreamType should be the last parameter before the options, so that the first three parameters are the same.

func NewServeMux(options ...MuxOption) (*http.ServeMux, error)

Would likely prefer we change this to http.Handler. The interface will never change, and (at least I) more clearly know what this is when reading this.

type AnyRequest
type AnyResponse

Just a note - when I first saw these, I thought google.protobuf.Any. These may make more sense as I read through the API, and I don't know if there's anything we can do about it.

type ClientOption

In addition to any options grouped in the documentation below, remember that Options are also valid ClientOptions.

Option inheritance? May make more sense as I get through this, but that scares me a bit.

    func WithRequestCompressor(name string) ClientOption

Can we strongly type the parameter with an enum? Or is this meant to be something that people can extend outside of this package (possible)?

type Code
    func CodeOf(err error) Code
    func (c Code) MarshalText() ([]byte, error)
    func (c Code) String() string
    func (c *Code) UnmarshalText(b []byte) error

I had more extensive godoc on these in https://github.com/bufbuild/buf/blob/main/private/pkg/rpc/errors.go#L24 - we don't need to directly copy/paste, but we might want to at least move the docs to above each type. I'm surprised we didn't get a lint warning on that (these aren't technically godoc'ed), although I think that was a golint-only thing.

Note that the specification uses the British "CANCELLED" for CodeCanceled.

Should we choose one and be consistent? I went with "CANCELED" in https://github.com/bufbuild/buf/blob/main/private/pkg/rpc/errors.go#L139.

type Doer

Bikeshedding, but I'm not crazy about Doer heh. I would prefer just HTTPClient personally, it's more clear to me what this does.

type Error
    func AsError(err error) (*Error, bool)
    func Errorf(c Code, template string, args ...any) *Error
    func Wrap(c Code, err error) *Error
    func (e *Error) AddDetail(d ErrorDetail)
    func (e *Error) Code() Code
    func (e *Error) Details() []ErrorDetail
    func (e *Error) Error() string
    func (e *Error) Unwrap() error

I really dislike when a struct is named Error due to its overlap with the builtin type. Especially with variable naming, i.e. you can't do error := connect.Errorf(...) But again, bikeshedding. I don't know. This is especially weird with i.e. the Wrap function, where it takes an error and returns a *Error. It's also weird that I can do *Error.Error().

I did the "builtin style" in https://github.com/bufbuild/buf/blob/main/private/pkg/rpc/errors.go where you only return error but that gets kind of wonky.

type ErrorDetail

I don't know if we should use protoreflect.FullName, rather just use a string.

Do we need UnmarshalTo?

type ErrorInterceptor
    func NewErrorInterceptor(toWire func(error) error, fromWire func(error) error) *ErrorInterceptor
    func (i *ErrorInterceptor) Wrap(next Func) Func
    func (i *ErrorInterceptor) WrapContext(ctx context.Context) context.Context
    func (i *ErrorInterceptor) WrapReceiver(_ context.Context, receiver Receiver) Receiver
    func (i *ErrorInterceptor) WrapSender(_ context.Context, sender Sender) Sender

func (error) error but works with *Errors? Head spinning. Need to understand this type more.

type Func

Don't love that we have NewClientFunc and then Func, see comments above.

type Handler
    func NewStreamingHandler(streamType StreamType, procedure, registrationName string, implementation func(context.Context, Sender, Receiver), options ...HandlerOption) (*Handler, error)
    func NewUnaryHandler[Req, Res any](procedure, registrationName string, unary func(context.Context, *Request[Req]) (*Response[Res], error), options ...HandlerOption) (*Handler, error)
    func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request)
type HandlerOption

Same comment about standardizing unary vs streaming naming, here specifically Stream vs Streaming. Also would move StreamType after procedure, registrationName.

type HeaderInterceptor
    func NewHeaderInterceptor(inspectRequestHeader func(Specification, http.Header), inspectResponseHeader func(Specification, http.Header)) *HeaderInterceptor
    func (h *HeaderInterceptor) Wrap(next Func) Func
    func (h *HeaderInterceptor) WrapContext(ctx context.Context) context.Context
    func (h *HeaderInterceptor) WrapReceiver(ctx context.Context, receiver Receiver) Receiver
    func (h *HeaderInterceptor) WrapSender(ctx context.Context, sender Sender) Sender

Want to understand the need for this and ErrorInterceptor more.

type Interceptor

I assume this is trying to provide a common interface for unary and streaming, is this right? Needed to scroll down to figure out UnaryInterceptorFunc a bit. Want to understand the interceptor setup a bit more, this is just a first pass. Not crazy about four functions.

type MuxOption
    func WithFallback(fallback http.Handler) MuxOption
    func WithHandlers(handlers []Handler, err error) MuxOption

type Option

The composition is throwing me off a bit, but...bikeshedding.

    func WithCodec(name string, c codec.Codec) Option
    func WithCompressor(name string, c compress.Compressor) Option

    func WithGRPC(enable bool) Option
    func WithGRPCWeb(enable bool) Option

Should we make boolean options have no parameters? This feels like a case where having Option be both a ClientOption and HandlerOption might result in suboptimal options.

    func WithInterceptors(interceptors ...Interceptor) Option

    func WithReadMaxBytes(n int64) Option

    func WithReplaceProcedurePrefix(prefix, replacement string) Option

type Receiver

Same error vs *Error comment.

Also general comment that there's a lot of client/server types with similar names imo (mux, handler, receiver, registrar)...need to understand this all a bit more but just first impression comment, probably a throwaway comment.

type Registrar
    func NewRegistrar() *Registrar
    func (r *Registrar) IsRegistered(service string) bool
    func (r *Registrar) Services() []string

This feeds into not using protoreflect.FullName - we use []string here.

type Request
    func NewRequest[Req any](msg *Req) *Request[Req]
    func ReceiveRequest[Req any](r Receiver) (*Request[Req], error)
    func (r *Request[_]) Any() any
    func (r *Request[_]) Header() http.Header
    func (r *Request[_]) Spec() Specification
type Response
    func NewResponse[Res any](msg *Res) *Response[Res]
    func ReceiveResponse[Res any](r Receiver) (*Response[Res], error)
    func (r *Response[_]) Any() any
    func (r *Response[_]) Header() http.Header
type Sender

type Specification

Might want to rename Type to StreamType.

type StreamType
type UnaryInterceptorFunc
    func (f UnaryInterceptorFunc) Wrap(next Func) Func
    func (f UnaryInterceptorFunc) WrapContext(ctx context.Context) context.Context
    func (f UnaryInterceptorFunc) WrapReceiver(_ context.Context, receiver Receiver) Receiver
    func (f UnaryInterceptorFunc) WrapSender(_ context.Context, sender Sender) Sender

We'd like users to be able to register their own compressors, both to support new algorithms (e.g., zstd) and faster implementations (e.g., github.com/klauspost/compress). Any new APIs should not use global registries or import-time side effects, and they should encourage compressors to use a sync.Pool.

I don't see much benefit in this, but grpc-go and Alex's branch both do it.

Package Structure

• The codec and compress packages are split out so that we can more easily add a separate package for the Protocol abstraction (e.g. gRPC, gRPC-Web, Connect) without introducing an import cycle.
• The clientstream and handlerstream packages are split out so that we can tidy up names for these types. Otherwise, you'd have names like NewClientClientStream (the client's view of a client streaming endpoint).
• We might want it to be compress.Gzip instead of compress.GzipCompressor.
  • Edit: We'll move this to compress/gzip.Compressor.

Method Naming

• The stream has a ReceivedHeader method to distinguish itself from the request headers. Should we instead just name this ResponseHeader for clarity?
• Similarly, let's make it explicit to be RequestHeader and ResponseHeader.
• As discussed, we need to decide what we're doing with Simple/Full.
  • Client-side: WrappedPingClient and UnwrappedPingClient interfaces. PingClient is reserved for the type that users interact with.
  • Server-side: PingService (acts upon generic types), and NewPingService (acts upon any). Comments are left in-line to describe how to implement the simple, non-generic method signatures.

Future Proofing

• Top-level abstractions (e.g. Codec and Compressor) are propagated through the clientCfg and into the protocol abstraction via a catch-all protocolClientParams. If we eventually plan to export the protocol abstraction and include it as a ClientOption, the relationship here is fuzzy.
  • What happens if we ever have a protocol that doesn't interact with all of the protocolClientParams types - is it a no-op, an error, or a silent failure?
  • We could tie these options to each protocol individually to clear these relationships up, but we end up with some more repetition (i.e. we need to repeat the same options for similar protocols like gRPC and gRPC-Web). For example, each of the gRPC and gRPC-Web protocols would have a Codec option.
  • In the connect-api branch, we were able to get around this because the protocols were separate client implementations, and they could each individually own what options they exposed (e.g. here).
• We still need to figure out error details. I know the gRPC protocol requires the proto.Any, and Akshay had some ideas around this - we could rename the methods to include Any so it leaves room for us to add other types later (e.g. AddAnyDetail). The abstraction violation between the pluggable Codec and the proto.Any sucks, but I know we're at mercy to the gRPC protocol here.

1.0 Features

• The gRPC health and reflection packages are valuable, but they aren't really necessary to begin with. We should consider whether or not these packages should exist in a separate repository (similar to gRPC middleware repositories).
  • I know we need to be mindful of this w.r.t. including the RegistrationName in the generated code. If we were to drop this support to begin with, we'd need to reintroduce this as a HandlerOption later, and that's tied to the connect library itself. It's not immediately obvious how this would work.
  • Decision: health and reflection are staying where they are. We need these features for easy gRPC user adoption. To be clear, health is non-optional. reflection is a huge quality of life improvement and its (nearly) part of the gRPC specification at this point.
• connect.MaxHeaderBytes is kinda nice, but doesn't feel necessary and is prone to change across different protocols.
• Should connect.ReceiveResponse be in an internal package? It's only used twice and otherwise distracts from the API that users ought to interact with. This might already be your plan based on the conversations we had earlier about the user-facing API and connect internals.
  • It looks like ReceiveRequest needs to be exported for the generated code, so I can see an argument to export it for symmetry.
• Drop IsValidHeaderKey and IsValidHeaderValue.

Implementation Details

• In the connect-api branch, I left a note for myself about whether or not the discard helper function can hang forever (e.g. discard(cs.response.Body)). This might have happened when I introduced the gRPC testing suite, but I can't recall. We need to make sure this can't happen.
  • Nothing to do here - this is a consequence of needing to read the http.Response.Body to completion to reuse the connection. This is also just an implementation detail, so it's not blocking regardless.

Right now, reRPC only exposes support for unary RPCs. However, it wouldn't be terribly difficult to support client, server, and bidirectional streaming - under the hood, NewReflectionHandler already uses bidi streaming.

Advantages of streaming:

• Support a wider variety of use cases.
• Users wouldn't need to migrate to grpc-go just to call or serve one streaming method, so building with reRPC becomes a safer bet.

Disadvantages of streaming:

• We'd be using net/http in a somewhat unusual way. I'd expect more difficult-to-diagnose bugs.
• reRPC's surface area would grow quite a bit. We'd need streaming equivalents for Func and Interceptor, and we might even need separate interfaces for each streaming type (like grpc-go).
• Twirp doesn't support streaming: twitchtv/twirp#3. Most of the discussion on that issue focuses on the JSON streaming format, but we'd also probably want a framing mechanism for binary protobuf and a way to communicate errors mid-stream (after the HTTP status code has already been sent). For all its complexity, the gRPC specification solves these problems well.
• Client and server streaming will work over HTTP/1.1, but bidi requires HTTP/2. With the lack of Twirp support above, this makes reRPC's compatibility guarantees harder to understand.

I lean toward keeping reRPC unary-only, but I'm leaving this issue open for discussion. I'm particularly interested in talking to anyone willing to use reRPC streaming in production.

We've implemented the gRPC family of protocols using an unexported protocol abstraction, with the thought that we might export that abstraction and allow third-party packages to implement protocols.

To do that, we'd of course need to be comfortable committing to backward compatibility for protocol interfaces. We also need to:

• Expose raw HTTP headers or dispense with validation completely. Protocols must be able to write reserved headers, and our ability to validate headers is limited if third-party packages can introduce new protocols (with new reserved headers).

There should only be one buf.work.yaml per repo - @amckinney we should talk about how this wasn't obvious.

We don't use them. If we have them, there actually are a few options I'd consider setting, but might break gRPC.

We also might want to just make a single instance protoBinaryCodec/protoJSONCodec as an immutable global as well, or make them not pointers.

A single interceptor should act at the same layer of the onion, regardless of whether a streaming or unary procedure is being called. This isn't controversial.

Today, the ordering for unary and streaming interceptors is effectively inverted - the outermost interceptor can access unary headers first, but streaming headers last. We should fix this.

The go-.* paradigm is really only used by protoc-gen-go-grpc, which is relatively new, and I'd argue that it's not productive. We generally want people to get into the habit of generating to a sub-directory named after their plugin, So we might want to have i.e /internal/gen/proto/goconnect or internal/gen/proto/connectgo, and then name the plugin accordingly.

Note that we did the same go-.* style with our internal plugins for bufbuild/buf (this is on me), so we should probably change that too once we agree on the naming scheme.

In line with getting rid of some of the files, would it make sense in either of these two? I assume the idea is that it won't be grpc-specific down the road. The big goal here is to not have a file named .*interceptor.* that isn't related to Interceptors.

I suspect that the current code gen doesn't handle comments and deprecation correctly - it probably produces invalid code. Alex's branch handles all this correctly, so we can pull any fixes required from there.

Not going to go through the whole codebase and update, just going forward...I'd propose 100, thoughts?

The current Stream interface is quite large, which increases the odds that we'll regret it. We should trim it down, possibly by splitting it into reader and writer interfaces.

We should implement the MaxReadBytes option with http.MaxBytesReader instead of the plain io.LimitReader - that's what it's for!

I think we should let users remix options. It's hard to do this from outside connect because the options interfaces have unexported functions.

func WithOptions(...Option) Option
func WithClientOptions(...ClientOption) ClientOption
func WithHandlerOptions(...HandlerOption) HandlerOption

Currently, the metadata (headers & footers) for client requests is only visible in interceptors. We should add CallOptions that make it easier to surface these to users. The API would likely be similar to grpc.Header and grpc.Footer.

It's convenient that we automatically handle and add codes to context.Canceled and context.DeadlineExceeded, but it's a strange special case. The code is also repetitive.

We should instead move this logic into an interceptor that users can opt into.

We should add a handful of benchmarks to the test suite (perhaps in the cross-tests so we can compare to grpc-go?). We should also do some local testing for more detailed throughput and latency measurement - fortio might be a good tool.

I believe right now call interceptors can access response headers only through an immutable view. Same for handler interceptors, but for request headers. I wonder if we should be more permissive. For example, a handler authentication interceptor might want to set the principal on the context and strip away authentication header info before forwarding the request to the next in chain.

The Stream interface looks like this:

type Stream interface {
        Context() context.Context
        Send(proto.Message) error
        CloseSend(error) error
        Receive(proto.Message) error
        CloseReceive() error
}

All errors that we encounter in the internals of reRPC are surfaced through this interface. However, in the generated code we hide some of these errors: for example, we call CloseSend on the user's behalf and don't have any way to log the returned error. We need to provide visibility into these errors.

I think that the right approach is to offer an interceptor-like API:

type Observer interface {
        Observe(Stream) Stream
}

Users can then log, collect metrics, push data to Sentry, or otherwise do as they please. We'd apply these observers to both streaming and unary calls.

This is a pretty annoying API, though - wrapping 5 methods just to get some logging and metrics seems irritating. Maybe there's a better way?

We want to encourage this method of adoption going forward, and it is strictly cleaner in my view. Generating grpc/twirp alongside go was always a hack IMO.

When I see WithGzip in the generated code for clients, I think "all my requests are being gzipped". However, this is WithGzipRequests, which also confuses me a bit - WithGzip says "this allows request and responses to be gzipped", so I'd think that WithGzipRequests allows just requests, but this isn't the case obviously.

I'd also wonder if we should just remove WithGzip altogether and call gzip support something that is standard, and then rename WithGzipRequests to WithGzip.

It's easy to unit test the implementations of protobuf services when the methods are unary: just call the method with the generated request struct. Now that we support streaming, testing is more complex.

Going through the whole HTTP layer is still an option, as we do in reRPC's own tests. It'd be nice to offer users a lighter-weight option, though - perhaps we could add rerpc.Pipe(), which would return a client-side stream and server-side stream? Users could then use the constructors in the generated code to build the stream types their business logic expects.

We should expose new APIs to make serialization pluggable. This lets us support new IDLs (e.g., flatbuffers), and it lets users opt into faster protobuf serializers (e.g., vtprotobuf).

It's not awesome to have connect.MuxOption and connect.HandlerOption - it'd definitely be nice and clean if we could unify them into one server-side option type without losing too much flexibility. Let's get some real miles on this and re-evaluate.

Before cutting v1, we should reconsider the client-side stream names: ServerStreamForClient is pretty bad, but I'm not sure what's better.

• Would prefer not underscores
• Maybe a "more normal" variable name for this

Tying together pluggable compressors (#32) and pluggable serialization (#32), we should be able to plug in whole protocols. Unless we're sure that we like the resulting API, we should keep it private for 1.0.

I've been using Travis on other projects, mostly out of inertia. This may be a good opportunity to use Actions, though.

The first-party gRPC implementations have a standardized battery of interoperability tests. Many of them test particular flags or option schemes that may not apply to us, but let's see if we can get some useful test coverage from them.

https://github.com/grpc/grpc/blob/master/doc/interop-test-descriptions.md

The current Twirp maintainers aren't interested in adding timeouts to the wire protocol (see twitchtv/twirp#329). They've done a remarkably good job keeping the protocol tiny, so I'm not surprised.

Currently, reRPC servers respect the Grpc-Timeout header, even when the content-type indicates that the client is using the Twirp protocol. This seems relatively harmless, but perhaps we should stick to the minimal spec and drop this support?

We should add server-side support for the grpc-web protocol. The protocol itself isn't too different from standard gRPC, and it'd be nice for reRPC servers to directly support web clients.

That said, I'm not sure how much traction grpc-web actually has (i.e., does anyone care about this?). I'm also unsure what our cross-testing options are - would we need to run a headless browser in the reRPC tests?

grpc-go lets user-defined error types marshal themselves to the wire by implementing interface { GRPCStatus() *statuspb.Status }. Should we support something similar?

Worth noting that this only works when marshaling to the wire format (unsurprisingly) - there's no mechanism to unmarshal the wire representation into a custom error type.