httptransport: improved errors API

go.mod

@@ -1,6 +1,6 @@
 module github.com/quay/clair/v4
 
-go 1.21.9
+go 1.22
 
 require (
 	github.com/Masterminds/semver v1.5.0

httptransport/error.go

@@ -1,89 +1,40 @@
 package httptransport
 
 import (
-	"bytes"
 	"context"
-	"encoding/json"
-	"errors"
 	"fmt"
 	"net/http"
 
-	"github.com/quay/zlog"
+	"github.com/quay/clair/v4/httptransport/internal/details"
 )
 
-// StatusClientClosedRequest is a nonstandard HTTP status code used when the
-// client has gone away.
-//
-// This convention is cribbed from Nginx.
-const statusClientClosedRequest = 499
-
-// ApiError writes an untyped (that is, "application/json") error with the
-// provided HTTP status code and message.
+// ApiError writes an error with the provided HTTP status code and message.
 //
 // ApiError does not return, but instead causes the goroutine to exit.
+//
+// Deprecated: This is implemented via [details.Error], which provides a
+// richer API.
 func apiError(ctx context.Context, w http.ResponseWriter, code int, f string, v ...interface{}) {
-	const errheader = `Clair-Error`
-	disconnect := false
-	select {
-	case <-ctx.Done():
-		disconnect = true
-	default:
-	}
-	if ev := zlog.Debug(ctx); ev.Enabled() {
-		ev.
-			Bool("disconnect", disconnect).
-			Int("code", code).
-			Str("error", fmt.Sprintf(f, v...)).
-			Msg("http error response")
-	} else {
-		ev.Send()
-	}
-	if disconnect {
-		// Exit immediately if there's no client to read the response, anyway.
-		w.WriteHeader(statusClientClosedRequest)
-		panic(http.ErrAbortHandler)
+	err := genericError{
+		status: code,
+		err:    fmt.Errorf(f, v...),
 	}
+	details.Error(ctx, w, &err)
+}
+
+type genericError struct {
+	status int
+	err    error
+}
 
-	h := w.Header()
-	h.Del("link")
-	h.Set("content-type", "application/json")
-	h.Set("x-content-type-options", "nosniff")
-	h.Set("trailer", errheader)
-	w.WriteHeader(code)
+func (e *genericError) Error() string {
+	return e.err.Error()
+}
 
-	var buf bytes.Buffer
-	buf.WriteString(`{"code":"`)
-	switch code {
-	case http.StatusBadRequest:
-		buf.WriteString("bad-request")
-	case http.StatusMethodNotAllowed:
-		buf.WriteString("method-not-allowed")
-	case http.StatusNotFound:
-		buf.WriteString("not-found")
-	case http.StatusTooManyRequests:
-		buf.WriteString("too-many-requests")
-	default:
-		buf.WriteString("internal-error")
-	}
-	buf.WriteByte('"')
-	if f != "" {
-		buf.WriteString(`,"message":`)
-		b, _ := json.Marshal(fmt.Sprintf(f, v...)) // OK use of encoding/json.
-		buf.Write(b)
-	}
-	buf.WriteByte('}')
+func (e *genericError) Unwrap() error {
+	return e.err
+}
 
-	if _, err := buf.WriteTo(w); err != nil {
-		h.Set(errheader, err.Error())
-	}
-	switch err := http.NewResponseController(w).Flush(); {
-	case errors.Is(err, nil):
-	case errors.Is(err, http.ErrNotSupported):
-		// Skip
-	default:
-		zlog.Warn(ctx).
-			Err(err).
-			Msg("unable to flush http response")
-	}
-	panic(http.ErrAbortHandler)
+func (e *genericError) ErrorStatus() int {
+	return e.status
 }

httptransport/error_test.go

@@ -52,7 +52,7 @@ func TestClientDisconnect(t *testing.T) {
 	}
 
 	<-handlerDone
-	if got, want := status, statusClientClosedRequest; got != want {
+	if got, want := status, 499; got != want {
 		t.Errorf("bad status code recorded: got: %d, want: %d", got, want)
 	}
 }

httptransport/internal/details/details.go

@@ -0,0 +1,198 @@
+// Package details contains helpers for implementing [RFC 9457], "Problem Details
+// for HTTP APIs."
+//
+// See the documentation on [Error] for how keys in the response are
+// constructed.
+//
+// [RFC 9457]: https://datatracker.ietf.org/doc/html/rfc9457
+package details
+
+import (
+	"context"
+	"errors"
+	"io"
+	"net/http"
+	"sync"
+
+	"github.com/quay/zlog"
+
+	"github.com/quay/clair/v4/internal/json"
+	"github.com/quay/clair/v4/internal/json/jsontext"
+)
+
+// StatusClientClosedRequest is a nonstandard HTTP status code used when the
+// client has gone away.
+//
+// This convention is cribbed from Nginx.
+const StatusClientClosedRequest = 499
+
+// ErrorTrailer contains errors encountered while writing the error response, if
+// any.
+const ErrorTrailer = `Clair-Error`
+
+// Default JSON encoding options.
+var opts = json.JoinOptions(json.DefaultOptionsV2())
+
+// Pool of JSON encoders.
+var encPool = sync.Pool{
+	New: func() any { return jsontext.NewEncoder(io.Discard) },
+}
+
+// Error constructs and sends a problem detail response, then causes the
+// goroutine to panic with [http.ErrAbortHandler]. Well-written handlers should
+// be structured to clean up or record events correctly in this instance.
+//
+// To customize the returned problem detail response, the error provided to this
+// function can provide any combination of the following methods:
+//
+//   - ErrorStatus() int
+//   - ErrorType() string
+//   - ErrorTitle() string
+//   - ErrorDetail() string
+//   - ErrorInstance() string
+//   - ErrorExtension() map[string]any
+//
+// The ErrorStatus method is always consulted and used for the HTTP response
+// code if present, otherwise [http.StatusInternalServerError] is used. All
+// other methods are used if present. If ErrorDetail is not provided, the value
+// of the Error method will be used instead.
+//
+// These methods correspond to the keys defined in RFC 9457, and so should
+// follow the guidance there. This means that the values returned by ErrorType
+// and ErrorInstance should be URIs if possible.
+func Error(ctx context.Context, w http.ResponseWriter, err error) {
+	disconnect := false
+	code := http.StatusInternalServerError
+	select {
+	case <-ctx.Done():
+		disconnect = true
+	default:
+	}
+	// Emit the log line in the defer path.
+	defer func() {
+		// If the client has disconnected, this will show up as a
+		// `disconnect=true, code=NNN` pair here and `code=499` in other HTTP
+		// metrics.
+		zlog.Debug(ctx).
+			Bool("disconnect", disconnect).
+			Int("code", code).
+			AnErr("error", err).
+			Msg("http error response")
+	}()
+
+	// Always check for the status code.
+	if i, ok := err.(errStatus); ok {
+		code = i.ErrorStatus()
+	}
+	// Exit immediately if there's no client to read the response.
+	if disconnect {
+		w.WriteHeader(StatusClientClosedRequest)
+		panic(http.ErrAbortHandler)
+	}
+
+	// The client is connected and presumably wants the error; configure the
+	// response headers.
+	h := w.Header()
+	h.Del("link")
+	h.Set("content-type", "application/problem+json")
+	h.Set("x-content-type-options", "nosniff")
+	h.Set("trailer", ErrorTrailer)
+	w.WriteHeader(code)
+
+	enc := encPool.Get().(*jsontext.Encoder)
+	defer func() { encPool.Put(enc) }()
+	enc.Reset(w, opts)
+
+	// Construct and write the details object in one pass.
+	wErr := func() error {
+		if err := enc.WriteToken(jsontext.ObjectStart); err != nil {
+			return err
+		}
+
+		var et errType
+		if errors.As(err, &et) {
+			if err := errors.Join(
+				enc.WriteValue(jsontext.Value(`"type"`)), enc.WriteToken(jsontext.String(et.ErrorType())),
+			); err != nil {
+				return err
+			}
+		}
+		var eti errTitle
+		if errors.As(err, &eti) {
+			if err := errors.Join(
+				enc.WriteValue(jsontext.Value(`"title"`)), enc.WriteToken(jsontext.String(eti.ErrorTitle())),
+			); err != nil {
+				return err
+			}
+		}
+
+		var detail string
+		var ed errDetail
+		if errors.As(err, &ed) {
+			detail = ed.ErrorDetail()
+		} else {
+			detail = err.Error()
+		}
+		if err := errors.Join(
+			enc.WriteValue(jsontext.Value(`"detail"`)), enc.WriteToken(jsontext.String(detail)),
+		); err != nil {
+			return err
+		}
+
+		var ei errInstance
+		if errors.As(err, &ei) {
+			if err := errors.Join(
+				enc.WriteValue(jsontext.Value(`"instance"`)), enc.WriteToken(jsontext.String(ei.ErrorInstance())),
+			); err != nil {
+				return err
+			}
+		}
+
+		var ee errExtension
+		if errors.As(err, &ee) {
+			for k, v := range ee.ErrorExtension() {
+				if err := errors.Join(
+					enc.WriteToken(jsontext.String(k)), json.MarshalEncode(enc, v, opts),
+				); err != nil {
+					return err
+				}
+			}
+		}
+
+		return enc.WriteToken(jsontext.ObjectEnd)
+	}()
+	if wErr != nil {
+		h.Set(ErrorTrailer, wErr.Error())
+	}
+
+	switch err := http.NewResponseController(w).Flush(); {
+	case errors.Is(err, nil):
+	case errors.Is(err, http.ErrNotSupported):
+		// Skip
+	default:
+		zlog.Warn(ctx).
+			Err(err).
+			Msg("unable to flush http response")
+	}
+
+	panic(http.ErrAbortHandler)
+}
+
+type errType interface {
+	ErrorType() string
+}
+type errStatus interface {
+	ErrorStatus() int
+}
+type errTitle interface {
+	ErrorTitle() string
+}
+type errDetail interface {
+	ErrorDetail() string
+}
+type errInstance interface {
+	ErrorInstance() string
+}
+type errExtension interface {
+	ErrorExtension() map[string]any
+}