Make Handler an http.Handler

We can reduce the number of new ideas this package introduces if we make
our Handler an actual http.Handler. This has a number of nice effects -
we can return the concrete type instead of an anonymous http.HandlerFunc
in a few places. Across the board, this commit also changes functions to
return the concrete http.ServeMux type instead of the http.Handler
interface.

bad_route.go

@@ -2,7 +2,6 @@ package rerpc
 
 import (
 	"context"
-	"net/http"
 
 	"google.golang.org/protobuf/proto"
 	"google.golang.org/protobuf/types/known/emptypb"
@@ -12,9 +11,10 @@ import (
 // standard library's http.StatusNotFound. To be fully compatible with the
 // Twirp specification, mount this handler at the root of your API (so that it
 // handles any requests for invalid protobuf methods).
-func NewBadRouteHandler(opts ...HandlerOption) http.Handler {
-	h := NewHandler(
+func NewBadRouteHandler(opts ...HandlerOption) *Handler {
+	return NewHandler(
 		"", "", "", // protobuf method, service, package names
+		func() proto.Message { return &emptypb.Empty{} }, // unused req msg
 		func(ctx context.Context, _ proto.Message) (proto.Message, error) {
 			path := "???"
 			if md, ok := HandlerMeta(ctx); ok {
@@ -24,7 +24,4 @@ func NewBadRouteHandler(opts ...HandlerOption) http.Handler {
 		},
 		opts...,
 	)
-	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		h.Serve(w, r, &emptypb.Empty{})
-	})
 }

cmd/protoc-gen-go-rerpc/rerpc.go

@@ -220,7 +220,7 @@ func serverConstructor(g *protogen.GeneratedFile, service *protogen.Service, nam
 		deprecated(g)
 	}
 	g.P("func New", service.GoName, "HandlerReRPC(svc ", name, ", opts ...", rerpcPackage.Ident("HandlerOption"),
-		") (string, ", httpPackage.Ident("Handler"), ") {")
+		") (string, *", httpPackage.Ident("ServeMux"), ") {")
 	g.P("mux := ", httpPackage.Ident("NewServeMux"), "()")
 	g.P()
 	for _, method := range unaryMethods(service) {
@@ -230,6 +230,7 @@ func serverConstructor(g *protogen.GeneratedFile, service *protogen.Service, nam
 		g.P(`"`, method.Desc.FullName(), `", // fully-qualified protobuf method`)
 		g.P(`"`, service.Desc.FullName(), `", // fully-qualified protobuf service`)
 		g.P(`"`, service.Desc.ParentFile().Package(), `", // fully-qualified protobuf package`)
+		g.P("func() ", protoPackage.Ident("Message"), " { return &", method.Input.GoIdent, "{} }, // request msg constructor")
 		g.P(rerpcPackage.Ident("Func"), "(func(ctx ", contextPackage.Ident("Context"),
 			", req ", protoPackage.Ident("Message"), ") (",
 			protoPackage.Ident("Message"), ", error) {")
@@ -245,9 +246,7 @@ func serverConstructor(g *protogen.GeneratedFile, service *protogen.Service, nam
 		g.P("}),")
 		g.P("opts...,")
 		g.P(")")
-		g.P(`mux.HandleFunc("/`, path, `", func(w `, httpPackage.Ident("ResponseWriter"), ", r *", httpPackage.Ident("Request"), ") {")
-		g.P(hname, ".Serve(w, r, &", method.Input.GoIdent, "{})")
-		g.P("})")
+		g.P(`mux.Handle("/`, path, `", `, hname, ")")
 		g.P()
 	}
 	comment(g, "Respond to unknown protobuf methods with gRPC and Twirp's 404 equivalents.")

handler.go

@@ -75,6 +75,7 @@ type Handler struct {
 	methodFQN      string
 	serviceFQN     string
 	packageFQN     string
+	newRequest     func() proto.Message
 	implementation Func
 	// rawGRPC is used only for our hand-rolled reflection handler, which needs
 	// bidi streaming
@@ -90,13 +91,24 @@ type Handler struct {
 }
 
 // NewHandler constructs a Handler. The supplied method, service, and package
-// must be fully-qualified protobuf identifiers.
+// must be fully-qualified protobuf identifiers, and the newRequest constructor
+// must be safe to call concurrently.
 //
 // For example, a handler might have method "acme.foo.v1.FooService.Bar",
-// service "acme.foo.v1.FooService", and package "acme.foo.v1". Remember that
-// NewHandler is usually called from generated code - most users won't need to
-// deal with protobuf identifiers directly.
-func NewHandler(methodFQN, serviceFQN, packageFQN string, impl Func, opts ...HandlerOption) *Handler {
+// service "acme.foo.v1.FooService", and package "acme.foo.v1". In that case,
+// the newRequest constructor would be:
+//   func() proto.Message {
+//     return &foopb.BarRequest{}
+//   }
+//
+// Remember that NewHandler is usually called from generated code - most users
+// won't need to deal with protobuf identifiers directly.
+func NewHandler(
+	methodFQN, serviceFQN, packageFQN string,
+	newRequest func() proto.Message,
+	impl Func,
+	opts ...HandlerOption,
+) *Handler {
 	var cfg handlerCfg
 	for _, opt := range opts {
 		opt.applyToHandler(&cfg)
@@ -108,19 +120,14 @@ func NewHandler(methodFQN, serviceFQN, packageFQN string, impl Func, opts ...Han
 		methodFQN:      methodFQN,
 		serviceFQN:     serviceFQN,
 		packageFQN:     packageFQN,
+		newRequest:     newRequest,
 		implementation: impl,
 		config:         cfg,
 	}
 }
 
-// Serve executes the handler, much like the standard library's http.Handler.
-// Unlike http.Handler, it requires a pointer to the generated request struct.
-// See the internal/ping/v1test package for an example of how this code is used
-// in reRPC's generated code.
-//
-// As long as the caller allocates a new request struct for each call, this
-// method is safe to call concurrently.
-func (h *Handler) Serve(w http.ResponseWriter, r *http.Request, req proto.Message) {
+// ServeHTTP implements http.Handler.
+func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	// To ensure that we can re-use connections, always consume and close the
 	// request body.
 	defer r.Body.Close()
@@ -250,7 +257,7 @@ func (h *Handler) Serve(w http.ResponseWriter, r *http.Request, req proto.Messag
 	} else {
 		implementation = h.implementationGRPC(w, r, spec)
 	}
-	res, err := h.wrap(implementation)(ctx, req)
+	res, err := h.wrap(implementation)(ctx, h.newRequest())
 	h.writeResult(r.Context(), w, spec, res, err)
 }
 

health.go

@@ -2,7 +2,6 @@ package rerpc
 
 import (
 	"context"
-	"fmt"
 	"net/http"
 
 	"google.golang.org/protobuf/proto"
@@ -63,17 +62,19 @@ func NewChecker(reg *Registrar) func(context.Context, string) (HealthStatus, err
 func NewHealthHandler(
 	check func(context.Context, string) (HealthStatus, error),
 	opts ...HandlerOption,
-) (string, http.Handler) {
+) (string, *http.ServeMux) {
 	const packageFQN = "grpc.health.v1"
 	const serviceFQN = packageFQN + ".Health"
 	const checkFQN = serviceFQN + ".Check"
 	const watchFQN = serviceFQN + ".Watch"
+	const servicePath = "/" + serviceFQN + "/"
+	const checkPath = servicePath + "Check"
+	const watchPath = servicePath + "Watch"
 
 	mux := http.NewServeMux()
 	checkHandler := NewHandler(
-		checkFQN,
-		serviceFQN,
-		packageFQN,
+		checkFQN, serviceFQN, packageFQN,
+		func() proto.Message { return &healthpb.HealthCheckRequest{} },
 		func(ctx context.Context, req proto.Message) (proto.Message, error) {
 			typed, ok := req.(*healthpb.HealthCheckRequest)
 			if !ok {
@@ -94,22 +95,18 @@ func NewHealthHandler(
 		},
 		opts...,
 	)
-	mux.HandleFunc(fmt.Sprintf("/%s/Check", serviceFQN), func(w http.ResponseWriter, r *http.Request) {
-		checkHandler.Serve(w, r, &healthpb.HealthCheckRequest{})
-	})
+	mux.Handle(checkPath, checkHandler)
 
 	watch := NewHandler(
-		watchFQN,
-		serviceFQN,
-		packageFQN,
+		watchFQN, serviceFQN, packageFQN,
+		func() proto.Message { return &healthpb.HealthCheckRequest{} },
 		func(ctx context.Context, req proto.Message) (proto.Message, error) {
 			return nil, errorf(CodeUnimplemented, "reRPC doesn't support watching health state")
 		},
 		opts...,
 	)
-	mux.HandleFunc(fmt.Sprintf("/%s/Watch", serviceFQN), func(w http.ResponseWriter, r *http.Request) {
-		watch.Serve(w, r, &healthpb.HealthCheckRequest{})
-	})
+	mux.Handle(watchPath, watch)
+	mux.Handle("/", NewBadRouteHandler(opts...))
 
-	return fmt.Sprintf("/%s/", serviceFQN), mux
+	return servicePath, mux
 }

reflection.go

@@ -83,24 +83,27 @@ func (r *Registrar) applyToHandler(cfg *handlerCfg) {
 //   https://github.com/grpc/grpc-go/blob/master/Documentation/server-reflection-tutorial.md
 //   https://github.com/grpc/grpc/blob/master/doc/server-reflection.md
 //   https://github.com/fullstorydev/grpcurl
-func NewReflectionHandler(reg *Registrar) (string, http.Handler) {
+func NewReflectionHandler(reg *Registrar) (string, *http.ServeMux) {
 	const packageFQN = "grpc.reflection.v1alpha"
 	const serviceFQN = packageFQN + ".ServerReflection"
-	const methodFQN = serviceFQN + ".ServerReflectionInfo"
+	const method = "ServerReflectionInfo"
+	const methodFQN = serviceFQN + "." + method
+	const servicePath = "/" + serviceFQN + "/"
+	const methodPath = servicePath + method
 	reg.register(serviceFQN)
 	h := NewHandler(
-		methodFQN,
-		serviceFQN,
-		packageFQN,
+		methodFQN, serviceFQN, packageFQN,
+		func() proto.Message { return nil },
 		nil,               // no unary implementation
 		ServeTwirp(false), // no reflection in Twirp
 	)
 	raw := &rawReflectionHandler{reg}
 	h.rawGRPC = raw.rawGRPC
-	httpHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		h.Serve(w, r, nil)
-	})
-	return "/grpc.reflection.v1alpha.ServerReflection/ServerReflectionInfo", httpHandler
+
+	mux := http.NewServeMux()
+	mux.Handle(methodPath, h)
+	mux.Handle("/", NewBadRouteHandler())
+	return servicePath, mux
 }
 
 type rawReflectionHandler struct {