Serve both image routes with one function and one error mapping (closes #316)

HandleImage parses the path and verifies the signature, and HandleImageEnc
decrypts the token. Each then calls serveImage, which chooses the format for
auto, gets the image and serves it. respondImageError answers the image
service's errors on both routes, and handleImageError is gone. /v1/image/ now
answers 504 when the upstream sends no response headers within
upstream_fetch_timeout, as README.md documented for /v1/e/. respondImageError
logs a 500 at error level and the other answers at warn. The image service
alone logs a source refused for its first bytes. A failed write is logged once,
at warn, with the request and the bytes served. README.md lists each image
route's answers as bullets. Tests send the same errors through both routes.

Model: opus-5-5

internal/handlers/avif_too_large_internal_test.go

@@ -12,11 +12,11 @@ import (
 	"sneak.berlin/go/pixa/internal/imagerequest"
 )
 
-// TestAVIFOutputTooLargeAnswers400 checks that respondImageError and
-// handleImageError, which answer the errors of the two image routes, answer
-// 400 naming the AVIF pixel limit when the image processor refuses an AVIF
-// output above it, wrapped as the service wraps it. The request's source only
-// goes into a log line, so it is left empty.
+// TestAVIFOutputTooLargeAnswers400 checks that respondImageError, which
+// answers the errors of both image routes, answers 400 naming the AVIF pixel
+// limit when the image processor refuses an AVIF output above it, wrapped as
+// the service wraps it. The request's source only goes into a log line, so it
+// is left empty.
 func TestAVIFOutputTooLargeAnswers400(t *testing.T) {
 	t.Parallel()
 
@@ -25,25 +25,15 @@ func TestAVIFOutputTooLargeAnswers400(t *testing.T) {
 		imageprocessor.ErrAVIFOutputTooLarge)
 	wantBody := fmt.Sprintf("above %d pixels", imageprocessor.MaxAVIFOutputPixels)
 
-	plain := httptest.NewRecorder()
-	h.respondImageError(plain, &imagerequest.Request{}, err)
+	rec := httptest.NewRecorder()
+	h.respondImageError(rec, &imagerequest.Request{}, err)
+	t.Logf("status = %d, body = %q", rec.Code, rec.Body.String())
 
-	encrypted := httptest.NewRecorder()
-	h.handleImageError(encrypted, err)
+	if rec.Code != http.StatusBadRequest {
+		t.Errorf("status = %d, want %d", rec.Code, http.StatusBadRequest)
+	}
 
-	for name, rec := range map[string]*httptest.ResponseRecorder{
-		"respondImageError": plain, "handleImageError": encrypted,
-	} {
-		t.Logf("%s: status = %d, body = %q", name, rec.Code, rec.Body.String())
-
-		if rec.Code != http.StatusBadRequest {
-			t.Errorf("%s: status = %d, want %d",
-				name, rec.Code, http.StatusBadRequest)
-		}
-
-		if !strings.Contains(rec.Body.String(), wantBody) {
-			t.Errorf("%s: body = %q, want it to say %q",
-				name, rec.Body.String(), wantBody)
-		}
+	if !strings.Contains(rec.Body.String(), wantBody) {
+		t.Errorf("body = %q, want it to say %q", rec.Body.String(), wantBody)
 	}
 }

internal/handlers/image.go

@@ -46,30 +46,7 @@ func (s *Handlers) HandleImage() http.HandlerFunc {
 			return
 		}
 
-		// The signature covers the format auto, not the format chosen
-		format, ok := s.chooseAutoFormat(w, r, req.Transform.Format)
-		if !ok {
-			return
-		}
-
-		req.Transform.Format = format
-
-		// Get cache key for logging
-		cacheKey := imagerequest.CacheKey(req)
-
-		// Get the image (from cache or fetch/process)
-		startTime := time.Now()
-
-		resp, err := s.imgSvc.Get(r.Context(), req)
-		if err != nil {
-			s.respondImageError(w, req, err)
-
-			return
-		}
-
-		defer func() { _ = resp.Content.Close() }()
-
-		s.writeImageResponse(w, r, req, expires, resp, cacheKey, startTime)
+		s.serveImage(w, r, req, expires)
 	}
 }
 
@@ -213,8 +190,8 @@ func (s *Handlers) validateSignature(
 }
 
 // parseExpires reads the exp query parameter, a Unix time in seconds. An exp
-// missing from the URL gives the zero time, which the signature check takes
-// as no expiration. An exp in the URL that is not a whole number, an empty
+// missing from the URL gives the zero time, which the signature check refuses
+// as a missing exp. An exp in the URL that is not a whole number, an empty
 // one included, is an error naming exp and the value.
 func parseExpires(query url.Values) (time.Time, error) {
 	if !query.Has("exp") {
@@ -242,7 +219,11 @@ func (s *Handlers) HandleTooManyRequests() http.HandlerFunc {
 	}
 }
 
-// respondImageError maps image retrieval errors to HTTP responses.
+// respondImageError answers err, the image service's error for req, on both
+// image routes. It logs a 500 at error level, as pixa's own failure. It logs
+// the other answers at warn level, since the client, the upstream or a full
+// limit caused them. The 415 for a source refused for its first bytes is not
+// logged here, because the image service logs it.
 func (s *Handlers) respondImageError(
 	w http.ResponseWriter, req *imagerequest.Request, err error,
 ) {
@@ -258,59 +239,53 @@ func (s *Handlers) respondImageError(
 		return
 	}
 
-	s.log.Error("failed to get image",
+	var (
+		status  int
+		message string
+	)
+
+	switch {
+	case errors.Is(err, httpfetcher.ErrSSRFBlocked):
+		status, message = http.StatusForbidden, "forbidden"
+	case errors.Is(err, httpfetcher.ErrUpstreamError):
+		status, message = http.StatusBadGateway, "upstream error"
+	case errors.Is(err, httpfetcher.ErrUpstreamTimeout):
+		status, message = http.StatusGatewayTimeout, "upstream timeout"
+	case errors.Is(err, httpfetcher.ErrTooManyConnections),
+		errors.Is(err, imageprocessor.ErrTooManyImages),
+		errors.Is(err, imgcache.ErrProcessingTimedOut):
+		status = http.StatusServiceUnavailable
+		message = "server busy, try again later"
+	case errors.Is(err, httpfetcher.ErrInvalidContentType),
+		errors.Is(err, imageprocessor.ErrUnsupportedInputFormat):
+		status = http.StatusUnsupportedMediaType
+		message = "unsupported source format"
+	case errors.Is(err, imageprocessor.ErrOutputTooLarge):
+		status = http.StatusBadRequest
+		message = fmt.Sprintf("size too large for the source: the side "+
+			"computed from its proportions is above %d",
+			imageprocessor.MaxOutputDimension)
+	case errors.Is(err, imageprocessor.ErrAVIFOutputTooLarge):
+		status = http.StatusBadRequest
+		message = fmt.Sprintf("size too large for AVIF: the output "+
+			"would be above %d pixels", imageprocessor.MaxAVIFOutputPixels)
+	default:
+		s.log.Error("failed to get image",
+			"host", req.Source.Host,
+			"path", req.Source.Path,
+			"error", err,
+		)
+		s.respondError(w, "internal error", http.StatusInternalServerError)
+
+		return
+	}
+
+	s.log.Warn("failed to get image",
 		"host", req.Source.Host,
 		"path", req.Source.Path,
 		"error", err,
 	)
-
-	// Check for specific error types
-	if errors.Is(err, httpfetcher.ErrSSRFBlocked) {
-		s.respondError(w, "forbidden", http.StatusForbidden)
-
-		return
-	}
-
-	if errors.Is(err, httpfetcher.ErrUpstreamError) {
-		s.respondError(w, "upstream error", http.StatusBadGateway)
-
-		return
-	}
-
-	if errors.Is(err, httpfetcher.ErrTooManyConnections) ||
-		errors.Is(err, imageprocessor.ErrTooManyImages) ||
-		errors.Is(err, imgcache.ErrProcessingTimedOut) {
-		s.respondError(w, "server busy, try again later",
-			http.StatusServiceUnavailable)
-
-		return
-	}
-
-	if errors.Is(err, httpfetcher.ErrInvalidContentType) ||
-		errors.Is(err, imageprocessor.ErrUnsupportedInputFormat) {
-		s.respondError(w, "unsupported source format",
-			http.StatusUnsupportedMediaType)
-
-		return
-	}
-
-	if errors.Is(err, imageprocessor.ErrOutputTooLarge) {
-		s.respondError(w, fmt.Sprintf("size too large for the source: the side "+
-			"computed from its proportions is above %d",
-			imageprocessor.MaxOutputDimension), http.StatusBadRequest)
-
-		return
-	}
-
-	if errors.Is(err, imageprocessor.ErrAVIFOutputTooLarge) {
-		s.respondError(w, fmt.Sprintf("size too large for AVIF: the output "+
-			"would be above %d pixels", imageprocessor.MaxAVIFOutputPixels),
-			http.StatusBadRequest)
-
-		return
-	}
-
-	s.respondError(w, "internal error", http.StatusInternalServerError)
+	s.respondError(w, message, status)
 }
 
 // cacheControl returns the Cache-Control header for an image served through a
@@ -367,14 +342,34 @@ func notModified(w http.ResponseWriter, r *http.Request, etag string) bool {
 	return true
 }
 
-// writeImageResponse writes headers and streams the image content,
-// handling conditional and HEAD requests. expires is the URL's expiry, which
-// ends the Cache-Control max-age.
-func (s *Handlers) writeImageResponse(
+// serveImage serves the image that req asks for. Both image routes call it
+// once they have the request and its URL's expiry, which ends the
+// Cache-Control max-age. It chooses the format for the format auto, answers
+// If-None-Match with 304 and HEAD with the headers only, and answers an error
+// of the image service with respondImageError.
+func (s *Handlers) serveImage(
 	w http.ResponseWriter, r *http.Request,
-	req *imagerequest.Request, expires time.Time, resp *imgcache.ImageResponse,
-	cacheKey imagerequest.VariantKey, startTime time.Time,
+	req *imagerequest.Request, expires time.Time,
 ) {
+	// The signature or the token covers the format auto, not the format chosen
+	format, ok := s.chooseAutoFormat(w, r, req.Transform.Format)
+	if !ok {
+		return
+	}
+
+	req.Transform.Format = format
+
+	startTime := time.Now()
+
+	resp, err := s.imgSvc.Get(r.Context(), req)
+	if err != nil {
+		s.respondImageError(w, req, err)
+
+		return
+	}
+
+	defer func() { _ = resp.Content.Close() }()
+
 	// Set response headers
 	w.Header().Set("Content-Type", resp.ContentType)
 
@@ -400,18 +395,28 @@ func (s *Handlers) writeImageResponse(
 	// Stream the response
 	w.WriteHeader(http.StatusOK)
 
+	// A failed write is most often a client that has gone away, not pixa's
+	// failure, so it is logged at warn level
 	servedBytes, err := io.Copy(w, resp.Content)
 	if err != nil {
-		s.log.Error("failed to write response",
+		s.log.Warn("failed to write response",
+			"request_id", middleware.GetReqID(r.Context()),
+			"host", req.Source.Host,
+			"path", req.Source.Path,
+			"served_bytes", servedBytes,
 			"error", err,
 		)
+
+		return
 	}
 
 	// Log cache status and timing after serving
 	duration := time.Since(startTime)
 	s.log.Info("image served",
 		"request_id", middleware.GetReqID(r.Context()),
-		"cache_key", cacheKey,
+		"cache_key", imagerequest.CacheKey(req),
+		"host", req.Source.Host,
+		"path", req.Source.Path,
 		"cache_status", resp.CacheStatus,
 		"duration_ms", duration.Milliseconds(),
 		"format", req.Transform.Format,

internal/handlers/image_errors_internal_test.go

@@ -1,18 +1,23 @@
 package handlers
 
 import (
+	"bytes"
+	"context"
 	"encoding/json"
 	"fmt"
 	"image/color"
+	"io"
 	"log/slog"
 	"net/http"
 	"net/http/httptest"
+	"slices"
 	"strings"
 	"testing"
 	"testing/fstest"
 	"time"
 
 	"github.com/go-chi/chi/v5"
+	"github.com/go-chi/chi/v5/middleware"
 	"github.com/prometheus/client_golang/prometheus"
 	"sneak.berlin/go/pixa/internal/allowlist"
 	"sneak.berlin/go/pixa/internal/httpfetcher"
@@ -414,3 +419,168 @@ func TestHandleImage_HashInSourcePathOrQuery_Returns400(t *testing.T) {
 		})
 	}
 }
+
+// failingFetcher answers every fetch with its error.
+type failingFetcher struct {
+	err error
+}
+
+// Fetch returns f's error.
+func (f failingFetcher) Fetch(
+	context.Context, imagerequest.Source,
+) (*httpfetcher.FetchResult, error) {
+	return nil, f.err
+}
+
+// photoURLsByRoute returns, for each image route, the function that makes the
+// URL of the JPEG at photoPath on signedHost on that route.
+func photoURLsByRoute() map[string]func(*testing.T, *Handlers) string {
+	return map[string]func(*testing.T, *Handlers) string{
+		"/v1/image/": signedPhotoURL,
+		"/v1/e/":     encPhotoURL,
+	}
+}
+
+// TestImageRoutes_SameErrorAnswers checks that both image routes answer each
+// upstream fetch error below with the same status. They log it as "failed to
+// get image" at the same level: error for the 500 and warn for the others.
+// Each request gets routes of its own, so no request finds another's failed
+// fetch in the negative cache.
+func TestImageRoutes_SameErrorAnswers(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name       string
+		err        error
+		wantStatus int
+		wantLevel  slog.Level
+	}{
+		{"blocked upstream address", httpfetcher.ErrSSRFBlocked,
+			http.StatusForbidden, slog.LevelWarn},
+		{"upstream error status",
+			fmt.Errorf("%w: status 404", httpfetcher.ErrUpstreamError),
+			http.StatusBadGateway, slog.LevelWarn},
+		{"no response headers within the fetch timeout",
+			httpfetcher.ErrUpstreamTimeout, http.StatusGatewayTimeout,
+			slog.LevelWarn},
+		{"no free upstream connection", httpfetcher.ErrTooManyConnections,
+			http.StatusServiceUnavailable, slog.LevelWarn},
+		{"Content-Type not a source format",
+			fmt.Errorf("%w: image/svg+xml", httpfetcher.ErrInvalidContentType),
+			http.StatusUnsupportedMediaType, slog.LevelWarn},
+		{"any other failure", io.ErrUnexpectedEOF,
+			http.StatusInternalServerError, slog.LevelError},
+	}
+
+	for _, tt := range tests {
+		for route, imageURL := range photoURLsByRoute() {
+			t.Run(route+" "+tt.name, func(t *testing.T) {
+				t.Parallel()
+
+				var logs bytes.Buffer
+
+				h, routes := newRoutesFetchingThrough(t,
+					failingFetcher{err: tt.err},
+					slog.New(slog.NewJSONHandler(&logs, nil)))
+
+				rec := sendGet(t, routes, imageURL(t, h))
+				if rec.Code != tt.wantStatus {
+					t.Errorf("status = %d, want %d: %s",
+						rec.Code, tt.wantStatus, rec.Body)
+				}
+
+				levels := loggedLevels(t, &logs)
+				if !slices.Contains(levels[tt.wantLevel.String()],
+					"failed to get image") {
+					t.Errorf(`no "failed to get image" line logged at level %s`,
+						tt.wantLevel)
+				}
+
+				if tt.wantLevel != slog.LevelError &&
+					len(levels[slog.LevelError.String()]) != 0 {
+					t.Errorf("logged at error level: %q",
+						levels[slog.LevelError.String()])
+				}
+			})
+		}
+	}
+}
+
+// failingResponseWriter is a response writer whose Write always fails, as when
+// the client has gone away.
+type failingResponseWriter struct {
+	*httptest.ResponseRecorder
+}
+
+// Write writes nothing and returns io.ErrClosedPipe.
+func (failingResponseWriter) Write([]byte) (int, error) {
+	return 0, io.ErrClosedPipe
+}
+
+// TestImageRoutes_FailedWriteLoggedOnce checks that both image routes log a
+// failed write of the image once, at warn level, with the request's ID, the
+// source host and path, the bytes served and the error. Neither may also log
+// the image as served.
+func TestImageRoutes_FailedWriteLoggedOnce(t *testing.T) {
+	t.Parallel()
+
+	const requestID = "test-request-id"
+
+	for route, imageURL := range photoURLsByRoute() {
+		t.Run(route, func(t *testing.T) {
+			t.Parallel()
+
+			var logs bytes.Buffer
+
+			h, srv := newSignedHostServer(t,
+				slog.New(slog.NewJSONHandler(&logs, nil)))
+
+			ctx := context.WithValue(t.Context(),
+				middleware.RequestIDKey, requestID)
+			srv.ServeHTTP(failingResponseWriter{httptest.NewRecorder()},
+				httptest.NewRequestWithContext(
+					ctx, http.MethodGet, imageURL(t, h), nil))
+
+			t.Logf("logged:\n%s", logs.String())
+
+			var failedWrites []map[string]any
+
+			dec := json.NewDecoder(&logs)
+			for dec.More() {
+				var line map[string]any
+
+				err := dec.Decode(&line)
+				if err != nil {
+					t.Fatalf("decoding log line: %v", err)
+				}
+
+				switch line["msg"] {
+				case "image served":
+					t.Error(`"image served" logged after a failed write`)
+				case "failed to write response":
+					failedWrites = append(failedWrites, line)
+				}
+			}
+
+			if len(failedWrites) != 1 {
+				t.Fatalf(`"failed to write response" logged %d times, want 1`,
+					len(failedWrites))
+			}
+
+			want := map[string]any{
+				"level":        slog.LevelWarn.String(),
+				"request_id":   requestID,
+				"host":         signedHost,
+				"path":         photoPath,
+				"served_bytes": float64(0),
+				"error":        io.ErrClosedPipe.Error(),
+			}
+
+			for key, wantValue := range want {
+				if got := failedWrites[0][key]; got != wantValue {
+					t.Errorf("%s = %v, want %v", key, got, wantValue)
+				}
+			}
+		})
+	}
+}

internal/handlers/imageenc.go

@@ -2,21 +2,13 @@ package handlers
 
 import (
 	"errors"
-	"fmt"
-	"io"
 	"net/http"
-	"strconv"
 	"time"
 
 	"github.com/go-chi/chi/v5"
-	"github.com/go-chi/chi/v5/middleware"
 
 	"sneak.berlin/go/pixa/internal/encurl"
-	"sneak.berlin/go/pixa/internal/httpfetcher"
-	"sneak.berlin/go/pixa/internal/imageprocessor"
 	"sneak.berlin/go/pixa/internal/imagerequest"
-	"sneak.berlin/go/pixa/internal/imgcache"
-	"sneak.berlin/go/pixa/internal/magic"
 )
 
 // HandleImageEnc handles requests to /v1/e/{token}/* for encrypted
@@ -28,83 +20,12 @@ func (s *Handlers) HandleImageEnc() http.HandlerFunc {
 			return
 		}
 
-		ctx := r.Context()
-		start := time.Now()
-
 		req, expires, ok := s.parseImageEncRequest(w, r)
 		if !ok {
 			return
 		}
 
-		format, ok := s.chooseAutoFormat(w, r, req.Transform.Format)
-		if !ok {
-			return
-		}
-
-		req.Transform.Format = format
-
-		// Log the request
-		s.log.Debug("encrypted image request",
-			"host", req.Source.Host,
-			"path", req.Source.Path,
-			"dimensions", fmt.Sprintf("%dx%d",
-				req.Transform.Size.Width, req.Transform.Size.Height),
-			"format", req.Transform.Format,
-		)
-
-		// Fetch and process the image (no signature validation
-		// needed - encrypted URL is trusted)
-		resp, err := s.imgSvc.Get(ctx, req)
-		if err != nil {
-			s.handleImageError(w, err)
-
-			return
-		}
-
-		defer func() { _ = resp.Content.Close() }()
-
-		// Set response headers
-		w.Header().Set("Content-Type", resp.ContentType)
-
-		if resp.ContentLength > 0 {
-			w.Header().Set("Content-Length", strconv.FormatInt(resp.ContentLength, 10))
-		}
-
-		// Cache headers: max-age ends at the URL's expiry
-		w.Header().Set("Cache-Control", cacheControl(expires))
-		w.Header().Set("X-Pixa-Cache", string(resp.CacheStatus))
-
-		if notModified(w, r, resp.ETag) {
-			return
-		}
-
-		// A HEAD request gets the headers only
-		if r.Method == http.MethodHead {
-			w.WriteHeader(http.StatusOK)
-
-			return
-		}
-
-		// Stream the response
-		written, err := io.Copy(w, resp.Content)
-		if err != nil {
-			s.log.Error("failed to write response", "error", err)
-
-			return
-		}
-
-		// Log completion
-		duration := time.Since(start)
-		s.log.Info("image served",
-			"request_id", middleware.GetReqID(ctx),
-			"cache_key", imagerequest.CacheKey(req),
-			"host", req.Source.Host,
-			"path", req.Source.Path,
-			"format", req.Transform.Format,
-			"cache_status", resp.CacheStatus,
-			"served_bytes", written,
-			"duration_ms", duration.Milliseconds(),
-		)
+		s.serveImage(w, r, req, expires)
 	}
 }
 
@@ -133,7 +54,7 @@ func (s *Handlers) parseImageEncRequest(
 			return nil, time.Time{}, false
 		}
 
-		s.log.Debug("failed to decrypt URL", "error", err)
+		s.log.Debug("invalid encrypted URL", "error", err)
 		s.respondError(w, "invalid encrypted URL", http.StatusBadRequest)
 
 		return nil, time.Time{}, false
@@ -141,8 +62,9 @@ func (s *Handlers) parseImageEncRequest(
 
 	// Apply the same dimension and fit-mode bounds as the plain image
 	// route: a sealed payload is trusted for its origin, not for staying
-	// within limits, so an over-limit size or unknown fit mode is a 400
-	// here rather than an out-of-memory or a 500 from the processor.
+	// within limits. This refuses an over-limit size before the source is
+	// fetched. An unknown fit mode is a 400 here rather than a 500 from the
+	// processor.
 	err = imagerequest.ValidateImageRequest(req)
 	if err != nil {
 		s.log.Debug("encrypted URL failed validation", "error", err)
@@ -154,38 +76,3 @@ func (s *Handlers) parseImageEncRequest(
 
 	return req, expires, true
 }
-
-// handleImageError converts image service errors to HTTP responses.
-func (s *Handlers) handleImageError(w http.ResponseWriter, err error) {
-	switch {
-	case errors.Is(err, httpfetcher.ErrSSRFBlocked):
-		s.respondError(w, "forbidden", http.StatusForbidden)
-	case errors.Is(err, httpfetcher.ErrUpstreamError):
-		s.respondError(w, "upstream error", http.StatusBadGateway)
-	case errors.Is(err, httpfetcher.ErrUpstreamTimeout):
-		s.respondError(w, "upstream timeout", http.StatusGatewayTimeout)
-	case errors.Is(err, httpfetcher.ErrTooManyConnections),
-		errors.Is(err, imageprocessor.ErrTooManyImages),
-		errors.Is(err, imgcache.ErrProcessingTimedOut):
-		s.respondError(w, "server busy, try again later",
-			http.StatusServiceUnavailable)
-	case errors.Is(err, httpfetcher.ErrInvalidContentType),
-		errors.Is(err, imageprocessor.ErrUnsupportedInputFormat),
-		errors.Is(err, magic.ErrMagicByteMismatch),
-		errors.Is(err, magic.ErrUnknownFormat),
-		errors.Is(err, magic.ErrNotEnoughData):
-		s.respondError(w, "unsupported source format",
-			http.StatusUnsupportedMediaType)
-	case errors.Is(err, imageprocessor.ErrOutputTooLarge):
-		s.respondError(w, fmt.Sprintf("size too large for the source: the side "+
-			"computed from its proportions is above %d",
-			imageprocessor.MaxOutputDimension), http.StatusBadRequest)
-	case errors.Is(err, imageprocessor.ErrAVIFOutputTooLarge):
-		s.respondError(w, fmt.Sprintf("size too large for AVIF: the output "+
-			"would be above %d pixels", imageprocessor.MaxAVIFOutputPixels),
-			http.StatusBadRequest)
-	default:
-		s.log.Error("image request failed", "error", err)
-		s.respondError(w, "internal error", http.StatusInternalServerError)
-	}
-}

internal/handlers/server_busy_internal_test.go

@@ -15,9 +15,10 @@ import (
 	"sneak.berlin/go/pixa/internal/imgcache"
 )
 
-// TestServerBusyAnswers503 checks that both image routes answer 503 with
-// a clear error when the image service gives up waiting for a free upstream
-// connection or processing slot, wrapped as the service wraps them.
+// TestServerBusyAnswers503 checks that respondImageError, which answers the
+// errors of both image routes, answers 503 with a clear error when the image
+// service gives up waiting for a free upstream connection or processing slot,
+// wrapped as the service wraps them.
 func TestServerBusyAnswers503(t *testing.T) {
 	t.Parallel()
 
@@ -30,35 +31,28 @@ func TestServerBusyAnswers503(t *testing.T) {
 		fmt.Errorf("upstream fetch failed: %w", httpfetcher.ErrTooManyConnections),
 		fmt.Errorf("image processing failed: %w", imageprocessor.ErrTooManyImages),
 	} {
-		plain := httptest.NewRecorder()
-		h.respondImageError(plain, req, err)
+		rec := httptest.NewRecorder()
+		h.respondImageError(rec, req, err)
 
-		encrypted := httptest.NewRecorder()
-		h.handleImageError(encrypted, err)
+		if rec.Code != http.StatusServiceUnavailable {
+			t.Errorf("for %v: status = %d, want %d",
+				err, rec.Code, http.StatusServiceUnavailable)
+		}
 
-		for route, rec := range map[string]*httptest.ResponseRecorder{
-			"/v1/image/": plain, "/v1/e/": encrypted,
-		} {
-			if rec.Code != http.StatusServiceUnavailable {
-				t.Errorf("%s for %v: status = %d, want %d",
-					route, err, rec.Code, http.StatusServiceUnavailable)
-			}
-
-			if !strings.Contains(rec.Body.String(), "server busy, try again later") {
-				t.Errorf("%s for %v: body = %q, want the server busy error",
-					route, err, rec.Body.String())
-			}
+		if !strings.Contains(rec.Body.String(), "server busy, try again later") {
+			t.Errorf("for %v: body = %q, want the server busy error",
+				err, rec.Body.String())
 		}
 	}
 }
 
-// TestProcessingTimedOutAnswers503 checks that respondImageError and
-// handleImageError, which answer the errors of the two image routes, answer
-// 503 with the server busy error when the image service reports that
-// processing stopped at the deadline of the request doing it, wrapped as the
-// service wraps it. A request that waited for that processing gets this error
-// while its own deadline may have time left. The request's source only goes
-// into a log line, so it is left empty.
+// TestProcessingTimedOutAnswers503 checks that respondImageError, which
+// answers the errors of both image routes, answers 503 with the server busy
+// error when the image service reports that processing stopped at the
+// deadline of the request doing it, wrapped as the service wraps it. A request
+// that waited for that processing gets this error while its own deadline may
+// have time left. The request's source only goes into a log line, so it is
+// left empty.
 func TestProcessingTimedOutAnswers503(t *testing.T) {
 	t.Parallel()
 
@@ -66,25 +60,15 @@ func TestProcessingTimedOutAnswers503(t *testing.T) {
 	err := fmt.Errorf("%w: failed to encode: %w",
 		imgcache.ErrProcessingTimedOut, context.DeadlineExceeded)
 
-	plain := httptest.NewRecorder()
-	h.respondImageError(plain, &imagerequest.Request{}, err)
+	rec := httptest.NewRecorder()
+	h.respondImageError(rec, &imagerequest.Request{}, err)
+	t.Logf("status = %d, body = %q", rec.Code, rec.Body.String())
 
-	encrypted := httptest.NewRecorder()
-	h.handleImageError(encrypted, err)
+	if rec.Code != http.StatusServiceUnavailable {
+		t.Errorf("status = %d, want %d", rec.Code, http.StatusServiceUnavailable)
+	}
 
-	for name, rec := range map[string]*httptest.ResponseRecorder{
-		"respondImageError": plain, "handleImageError": encrypted,
-	} {
-		t.Logf("%s: status = %d, body = %q", name, rec.Code, rec.Body.String())
-
-		if rec.Code != http.StatusServiceUnavailable {
-			t.Errorf("%s: status = %d, want %d",
-				name, rec.Code, http.StatusServiceUnavailable)
-		}
-
-		if !strings.Contains(rec.Body.String(), "server busy, try again later") {
-			t.Errorf("%s: body = %q, want the server busy error",
-				name, rec.Body.String())
-		}
+	if !strings.Contains(rec.Body.String(), "server busy, try again later") {
+		t.Errorf("body = %q, want the server busy error", rec.Body.String())
 	}
 }

internal/handlers/unsupported_source_internal_test.go

@@ -13,10 +13,11 @@ import (
 	"sneak.berlin/go/pixa/internal/imagerequest"
 )
 
-// TestUnsupportedSourceFormatAnswers415 checks that both image routes answer
-// 415 with a clear error when the upstream answers with a content type pixa
-// does not accept, such as SVG, and when orig is asked of a source in a
-// format the processor cannot name, wrapped as the service wraps them.
+// TestUnsupportedSourceFormatAnswers415 checks that respondImageError, which
+// answers the errors of both image routes, answers 415 with a clear error when
+// the upstream answers with a content type pixa does not accept, such as SVG,
+// and when orig is asked of a source in a format the processor cannot name,
+// wrapped as the service wraps them.
 func TestUnsupportedSourceFormatAnswers415(t *testing.T) {
 	t.Parallel()
 
@@ -31,26 +32,18 @@ func TestUnsupportedSourceFormatAnswers415(t *testing.T) {
 		fmt.Errorf("image processing failed: %w: unknown",
 			imageprocessor.ErrUnsupportedInputFormat),
 	} {
-		plain := httptest.NewRecorder()
-		h.respondImageError(plain, req, err)
+		rec := httptest.NewRecorder()
+		h.respondImageError(rec, req, err)
+		t.Logf("for %v: %d %s", err, rec.Code, rec.Body.String())
 
-		encrypted := httptest.NewRecorder()
-		h.handleImageError(encrypted, err)
+		if rec.Code != http.StatusUnsupportedMediaType {
+			t.Errorf("for %v: status = %d, want %d",
+				err, rec.Code, http.StatusUnsupportedMediaType)
+		}
 
-		for route, rec := range map[string]*httptest.ResponseRecorder{
-			"HandleImage": plain, "HandleImageEnc": encrypted,
-		} {
-			t.Logf("%s for %v: %d %s", route, err, rec.Code, rec.Body.String())
-
-			if rec.Code != http.StatusUnsupportedMediaType {
-				t.Errorf("%s for %v: status = %d, want %d",
-					route, err, rec.Code, http.StatusUnsupportedMediaType)
-			}
-
-			if !strings.Contains(rec.Body.String(), "unsupported source format") {
-				t.Errorf("%s for %v: body = %q, want the unsupported source format error",
-					route, err, rec.Body.String())
-			}
+		if !strings.Contains(rec.Body.String(), "unsupported source format") {
+			t.Errorf("for %v: body = %q, want the unsupported source format error",
+				err, rec.Body.String())
 		}
 	}
 }