feat(handler): Implement per-method timeouts with proto extensions

[huyquanha]

Context

See issue: #879

Intent

This implementation draws inspirations from the grpc-gateway openapiv2 options. Users can specify a method-level option (like idempotency_level, only difference is this one isn't a built-in field) to set custom read/write timeouts for that handler method.

import "connectrpc/go/options/v1/annotations.proto";

service GreetService {
  rpc Greet(Request) returns (Response) {
    option (connectrpc.go.options.v1.timeouts) = {
      // custom timeouts that can be different from server-wide timeouts.
      read_ms: 1000
      write_ms: 2000
    };
  }

  rpc GreetStream(stream GreetRequest) returns (stream GreetResponse) {
     option (connectrpc.go.options.v1.timeouts) = {
      // disable server-wide timeouts because it's a streaming rpc.
      read_ms: -1
      write_ms: -1
    };
  }
}

protoc-gen-connect-go then injects the timeouts into the generated *connect.go file, which're propgated to the handler ServeHttp method, where we can invoke responseController.SetRead/WriteDeadline to set per-handler timeouts.

An example generated code look like this

    testServiceMethod1Handler := connect.NewUnaryHandler(
		TestServiceMethod1Procedure,
		svc.Method1,
		connect.WithSchema(testServiceMethods.ByName("Method1")),
		connect.WithReadTimeout(time.Duration(1000)*time.Millisecond),
		connect.WithWriteTimeout(time.Duration(2000)*time.Millisecond),
		connect.WithHandlerOptions(opts...),
	)

The following rules apply:

• If the timeout value is < 0, we set the deadline to zero-value time.Time{} i.e. no deadline. This follows the same convention as http.Server timeouts, and is most beneficial for streaming endpoints where we want to disable timeouts, while still let users define server-wide timeouts to protect non-streaming RPCs.
• If the timeout value is 0, we don't set any deadline. The server-wide timeouts (if defined) takes effect
• If the timeout value > 0, we calculate the deadline as usual and set it.

Notes

• If this approach sounds reasonable to you, we will need to get a unique ID for this project, so we can use as the extension field number and avoid conflicts (see comment)

• I noticed there's another PR attempting to implement the same feature, however it only takes care of the handler-side logic, without a mechanism to let users inject these timeouts into each handler easily.

[huyquanha]

A name is needed so we can buf push it eventually.

I'm not that much of a Buf expert so not really sure if the lint/breaking rules make sense, I simply copied from the root buf.yaml.

[huyquanha]

presumably we could also add all the other testdata modules here too, but it's not strictly required right now.

[jhump]

@huyquanha, while this is a neat idea, it does not belong in this core repo. This is too opinionated about the means of configuration and adds too much machinery into the core. IMO, this sort of extension of the core connect-go functionality really belongs in a 3rd-party library. Even if we in the connectrpc organization wanted to offer it, it would be a separate repo, not baked into this core repo.

So I think you'd be best off for now moving this code into a 3rd-party library. You can create a different package hierarchy for the Protobuf files based on it living in a separate organization (and then you can push the definitions to buf.build from that other repo). And you can also reserve your own custom option number from the Protobuf registry. As far as the actual behavior of applying the timeouts, you should be able to do this from an interceptor. An accessor/constructor for this interceptor would likely be the only API needed in this 3rd-party library. Instead of augmenting the protoc-gen-connect-go code generator, the interceptor could determine the timeouts using Protobuf reflection, querying the method descriptors for these custom options.

[huyquanha]

Hey @jhump 👋 Thanks for the suggestion!

However, I'm not sure the interceptor approach will work atm. There's no way, afaict, to get access to the ResponseWriter/ResponseController to change the underlying net.Conn timeout from an interceptor. A very common use case for this feature is users setting some (small) server-wide read/write timeout, and then changing only the streaming RPCs timeouts to be higher/or disable it even. With an interceptor, the best we can do is setting a timeout on the context, which isn't really the same thing.

[emcfarlane]

Hey @huyquanha, you may find the https://pkg.go.dev/connectrpc.com/authn package as a useful reference. It wraps as HTTP middleware to ensure auth is handled before message decoding. It also needs to infer the protocol. This technique could be used to apply the timeouts on the ResponseController before invoking the connect-go handler. Otherwise you can also propagate the ResponseController through the context and apply timeouts in the interceptor.