tsweb: rename Handler to ReturnHandler

The name's been bugging me for a long time.

I liked neither the overlap between tsweb.Handler and http.Handler,
nor the name "ServeHTTPErr" which sounds like it's an error being
returned, like it's an error handler and not sometimes a happy path.

Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>

cmd/microproxy/microproxy.go

@@ -95,7 +95,7 @@ func promPrint(w io.Writer, prefix string, obj map[string]interface{}) {
 	}
 }
 
-func (h *goVarsHandler) ServeHTTPErr(w http.ResponseWriter, r *http.Request) error {
+func (h *goVarsHandler) ServeHTTPReturn(w http.ResponseWriter, r *http.Request) error {
 	resp, err := http.Get(h.url)
 	if err != nil {
 		return tsweb.Error(http.StatusInternalServerError, "fetch failed", err)

tsweb/tsweb.go

@@ -141,35 +141,26 @@ func stripPort(hostport string) string {
 	return net.JoinHostPort(host, "443")
 }
 
-// Handler is like net/http.Handler, but the handler can return an
+// ReturnHandler is like net/http.Handler, but the handler can return an
 // error instead of writing to its ResponseWriter.
-type Handler interface {
+type ReturnHandler interface {
-	// ServeHTTPErr is like http.Handler.ServeHTTP, except that
+	// ServeHTTPReturn is like http.Handler.ServeHTTP, except that
 	// it can choose to return an error instead of writing to its
 	// http.ResponseWriter.
 	//
-	// If ServeHTTPErr returns an error, it caller should handle
+	// If ServeHTTPReturn returns an error, it caller should handle
 	// an error by serving an HTTP 500 response to the user. The
 	// error details should not be sent to the client, as they may
 	// contain sensitive information. If the error is an
 	// HTTPError, though, callers should use the HTTP response
 	// code and message as the response to the client.
-	ServeHTTPErr(http.ResponseWriter, *http.Request) error
+	ServeHTTPReturn(http.ResponseWriter, *http.Request) error
 }
 
-// HandlerFunc is an adapter to allow the use of ordinary functions as
+// StdHandler converts a ReturnHandler into a standard http.Handler.
-// Handlers. If f is a function with the appropriate signature,
-// HandlerFunc(f) is a Handler that calls f.
-type HandlerFunc func(http.ResponseWriter, *http.Request) error
-
-func (h HandlerFunc) ServeHTTPErr(w http.ResponseWriter, r *http.Request) error {
-	return h(w, r)
-}
-
-// StdHandler converts a Handler into a standard http.Handler.
 // Handled requests are logged using logf, as are any errors. Errors
 // are handled as specified by the Handler interface.
-func StdHandler(h Handler, logf logger.Logf) http.Handler {
+func StdHandler(h ReturnHandler, logf logger.Logf) http.Handler {
 	return stdHandler(h, logf, time.Now, true)
 }
 
@@ -177,24 +168,24 @@ func StdHandler(h Handler, logf logger.Logf) http.Handler {
 // requests don't write an access log entry to logf.
 //
 // TODO(danderson): quick stopgap, probably want ...Options on StdHandler instead?
-func StdHandlerNo200s(h Handler, logf logger.Logf) http.Handler {
+func StdHandlerNo200s(h ReturnHandler, logf logger.Logf) http.Handler {
 	return stdHandler(h, logf, time.Now, false)
 }
 
-func stdHandler(h Handler, logf logger.Logf, now func() time.Time, log200s bool) http.Handler {
+func stdHandler(h ReturnHandler, logf logger.Logf, now func() time.Time, log200s bool) http.Handler {
-	return handler{h, logf, now, log200s}
+	return retHandler{h, logf, now, log200s}
 }
 
-// handler is an http.Handler that wraps a Handler and handles errors.
+// retHandler is an http.Handler that wraps a Handler and handles errors.
-type handler struct {
+type retHandler struct {
-	h       Handler
+	rh      ReturnHandler
 	logf    logger.Logf
 	timeNow func() time.Time
 	log200s bool
 }
 
 // ServeHTTP implements the http.Handler interface.
-func (h handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
+func (h retHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	msg := AccessLogRecord{
 		When:       h.timeNow(),
 		RemoteAddr: r.RemoteAddr,
@@ -208,7 +199,7 @@ func (h handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	}
 
 	lw := &loggingResponseWriter{ResponseWriter: w, logf: h.logf}
-	err := h.h.ServeHTTPErr(lw, r)
+	err := h.rh.ServeHTTPReturn(lw, r)
 	hErr, hErrOK := err.(HTTPError)
 
 	if lw.code == 0 && err == nil && !lw.hijacked {
@@ -318,7 +309,7 @@ func (l loggingResponseWriter) Flush() {
 
 // HTTPError is an error with embedded HTTP response information.
 //
-// It is the error type to be (optionally) used by Handler.ServeHTTPErr.
+// It is the error type to be (optionally) used by Handler.ServeHTTPReturn.
 type HTTPError struct {
 	Code int    // HTTP response code to send to client; 0 means means 500
 	Msg  string // Response body to send to client

tsweb/tsweb_test.go

@@ -29,16 +29,22 @@ func (h *noopHijacker) Hijack() (net.Conn, *bufio.ReadWriter, error) {
 	return nil, nil, nil
 }
 
+type handlerFunc func(http.ResponseWriter, *http.Request) error
+
+func (f handlerFunc) ServeHTTPReturn(w http.ResponseWriter, r *http.Request) error {
+	return f(w, r)
+}
+
 func TestStdHandler(t *testing.T) {
 	var (
-		handlerCode = func(code int) Handler {
+		handlerCode = func(code int) ReturnHandler {
-			return HandlerFunc(func(w http.ResponseWriter, r *http.Request) error {
+			return handlerFunc(func(w http.ResponseWriter, r *http.Request) error {
 				w.WriteHeader(code)
 				return nil
 			})
 		}
-		handlerErr = func(code int, err error) Handler {
+		handlerErr = func(code int, err error) ReturnHandler {
-			return HandlerFunc(func(w http.ResponseWriter, r *http.Request) error {
+			return handlerFunc(func(w http.ResponseWriter, r *http.Request) error {
 				if code != 0 {
 					w.WriteHeader(code)
 				}
@@ -66,14 +72,14 @@ func TestStdHandler(t *testing.T) {
 
 	tests := []struct {
 		name     string
-		h        Handler
+		rh       ReturnHandler
 		r        *http.Request
 		wantCode int
 		wantLog  AccessLogRecord
 	}{
 		{
 			name:     "handler returns 200",
-			h:        handlerCode(200),
+			rh:       handlerCode(200),
 			r:        req(bgCtx, "http://example.com/"),
 			wantCode: 200,
 			wantLog: AccessLogRecord{
@@ -90,7 +96,7 @@ func TestStdHandler(t *testing.T) {
 
 		{
 			name:     "handler returns 404",
-			h:        handlerCode(404),
+			rh:       handlerCode(404),
 			r:        req(bgCtx, "http://example.com/foo"),
 			wantCode: 404,
 			wantLog: AccessLogRecord{
@@ -106,7 +112,7 @@ func TestStdHandler(t *testing.T) {
 
 		{
 			name:     "handler returns 404 via HTTPError",
-			h:        handlerErr(0, Error(404, "not found", testErr)),
+			rh:       handlerErr(0, Error(404, "not found", testErr)),
 			r:        req(bgCtx, "http://example.com/foo"),
 			wantCode: 404,
 			wantLog: AccessLogRecord{
@@ -123,7 +129,7 @@ func TestStdHandler(t *testing.T) {
 
 		{
 			name:     "handler returns 404 with nil child error",
-			h:        handlerErr(0, Error(404, "not found", nil)),
+			rh:       handlerErr(0, Error(404, "not found", nil)),
 			r:        req(bgCtx, "http://example.com/foo"),
 			wantCode: 404,
 			wantLog: AccessLogRecord{
@@ -139,7 +145,7 @@ func TestStdHandler(t *testing.T) {
 
 		{
 			name:     "handler returns generic error",
-			h:        handlerErr(0, testErr),
+			rh:       handlerErr(0, testErr),
 			r:        req(bgCtx, "http://example.com/foo"),
 			wantCode: 500,
 			wantLog: AccessLogRecord{
@@ -156,7 +162,7 @@ func TestStdHandler(t *testing.T) {
 
 		{
 			name:     "handler returns error after writing response",
-			h:        handlerErr(200, testErr),
+			rh:       handlerErr(200, testErr),
 			r:        req(bgCtx, "http://example.com/foo"),
 			wantCode: 200,
 			wantLog: AccessLogRecord{
@@ -173,7 +179,7 @@ func TestStdHandler(t *testing.T) {
 
 		{
 			name:     "handler returns HTTPError after writing response",
-			h:        handlerErr(200, Error(404, "not found", testErr)),
+			rh:       handlerErr(200, Error(404, "not found", testErr)),
 			r:        req(bgCtx, "http://example.com/foo"),
 			wantCode: 200,
 			wantLog: AccessLogRecord{
@@ -190,7 +196,7 @@ func TestStdHandler(t *testing.T) {
 
 		{
 			name:     "handler does nothing",
-			h:        HandlerFunc(func(http.ResponseWriter, *http.Request) error { return nil }),
+			rh:       handlerFunc(func(http.ResponseWriter, *http.Request) error { return nil }),
 			r:        req(bgCtx, "http://example.com/foo"),
 			wantCode: 200,
 			wantLog: AccessLogRecord{
@@ -206,7 +212,7 @@ func TestStdHandler(t *testing.T) {
 
 		{
 			name: "handler hijacks conn",
-			h: HandlerFunc(func(w http.ResponseWriter, r *http.Request) error {
+			rh: handlerFunc(func(w http.ResponseWriter, r *http.Request) error {
 				_, _, err := w.(http.Hijacker).Hijack()
 				if err != nil {
 					t.Errorf("couldn't hijack: %v", err)
@@ -241,7 +247,7 @@ func TestStdHandler(t *testing.T) {
 			clock.Reset()
 
 			rec := noopHijacker{httptest.NewRecorder(), false}
-			h := stdHandler(test.h, logf, clock.Now, true)
+			h := stdHandler(test.rh, logf, clock.Now, true)
 			h.ServeHTTP(&rec, test.r)
 			res := rec.Result()
 			if res.StatusCode != test.wantCode {