feat(errs): http error classifier (#566)

## What

platform/errs treats an unclassified error as non-retryable, so every
HTTP failure an extension returned took that default. A 502 from a proxy
in front of a build API was indistinguishable from "this request is
invalid": the message dead-lettered on its first attempt instead of
being retried.

Add `platform/http.StatusError`, a typed error that keeps the status
code in the chain, and `platform/errs/http`, the classifier that reads
it. Server-state codes (500, 502, 503, 504, other 5xx, 429, 408) are
retryable dependency errors; verdicts on the request (4xx, 3xx, the
permanently broken 501 and 505, and a code that was never a response)
are not.

`*url.Error` covers transport failures, including an expired deadline —
that means the remote end did not answer in time, so it is attributed to
the dependency. Only `context.Canceled` is declined, so a shutdown stays
out of a backend's dependency metrics and `platform/errs/generic` claims
that node. Nothing in the classifier depends on `platform/errs/mysql`
being wired to reach a verdict; a test pins the `generic` + `http`
wiring a service with no MySQL dependency would use.

The classifier must still be listed before `platform/errs/mysql`, whose
`net.Error` rule matches `*url.Error` and would otherwise claim HTTP
transport failures. Documented in the errs README next to the wiring
example.

`StatusError.Error` renders at most 1 KiB of the response body. The
rendered string lands in the queue's dead-letter record via `Reject(ctx,
err.Error())`, and `last_error` is a finite column, so an error page
from a chatty gateway must not be able to fail that write and leave the
message stuck rather than dead-lettered. `Body` itself is kept whole for
callers that want to inspect it.

Convert both build-runner clients — Buildkite and GitHub Actions — to
return the typed error, and wire the classifier into the stovepipe
server, which runs both. Client tests now assert the code is reachable
with `errors.As`, so a refactor back to `fmt.Errorf` cannot pass
silently. The submitqueue-side clients (`changeprovider/github`,
`phabricator/conduit`, `mergechecker/github`) are unchanged: no service
wires `httperrs` on those paths, so nothing would read the code.

## Behavior change worth calling out: retried creates can duplicate a
build

Creating a build is not idempotent, and a rejected create is now
reported with its status code like any other, so a retried 502 can start
a second build when the first was already accepted. Stovepipe's build
controller has no guard against that — `request.State.IsTerminal()` is
false at the point `Trigger` runs, and the `ErrAlreadyExists` tolerance
keys on a build ID the retry has not minted yet.

This exposure is not new: a transport failure on the same call is
already retryable today through the mysql classifier's `net.Error` rule,
so a connection reset mid-write can already duplicate a build. This
change widens it from transport failures to 5xx responses. The trade is
deliberate — a request that gives up on a proxy blip strands its queue
slot, which is worse — and both clients' create paths now document it.
Making create idempotent is follow-up work and is the next branch.

## Why

- Correctly classify various types of HTTP errors as retryable

## Test Plan

- Deploy with corresponding changes internally
- Monitor DLQ volume in the message queue — retryable failures should
stop landing there
- Watch build creation counts against request counts for the
duplicate-build case above, which DLQ volume will not show

Author: mnoah1

platform/errs/README.md

@@ -58,7 +58,7 @@ Two implementations ship in this package:
 
 ## Adding a Backend-Specific Classifier
 
-Backend classifiers live alongside the extension they classify, under `platform/errs/<backend>/`. The canonical examples are `platform/errs/mysql` (MySQL driver errors) and `platform/errs/generic` (transport-agnostic concerns such as `context.Canceled`).
+Backend classifiers live alongside the extension they classify, under `platform/errs/<backend>/`. The canonical examples are `platform/errs/mysql` (MySQL driver errors), `platform/errs/http` (rejected status codes and transport failures from clients built on `platform/http`), and `platform/errs/generic` (transport-agnostic concerns such as `context.Canceled`).
 
 A classifier:
 
@@ -91,17 +91,21 @@ Servers wire each classifier into the consumer's `ErrorProcessor`. Order matters
 import (
     "github.com/uber/submitqueue/platform/errs"
     genericerrs "github.com/uber/submitqueue/platform/errs/generic"
+    httperrs    "github.com/uber/submitqueue/platform/errs/http"
     mysqlerrs   "github.com/uber/submitqueue/platform/errs/mysql"
 )
 
 c := consumer.New(logger, scope, registry,
     errs.NewClassifierProcessor(
         genericerrs.Classifier,
+        httperrs.Classifier,
         mysqlerrs.Classifier,
     ),
 )
 ```
 
+`httperrs` precedes `mysqlerrs` for a reason worth knowing before reordering the list: the MySQL classifier treats any `net.Error` as retryable infra, and the `*url.Error` an HTTP client returns satisfies `net.Error`. Whichever runs first claims that node, so with the order reversed an HTTP transport failure is classified as a MySQL one — retryable either way, but no longer attributed to the dependency it came from. This is the cross-extension ambiguity `NewClassifierProcessor` documents as deferred; registration order is the workaround.
+
 Tests follow the same shape: assert per-node behaviour against `Classifier.Classify(node)` directly, and assert end-to-end behaviour by running `errs.NewClassifierProcessor(Classifier).Process(err)` and checking the helpers (`IsRetryable`, `IsUserError`, …) on the result. See `platform/errs/mysql/mysql_test.go` and `platform/errs/generic/generic_test.go`.
 
 ## Overriding Classification from a Controller

platform/errs/http/BUILD.bazel

@@ -0,0 +1,25 @@
+load("@rules_go//go:def.bzl", "go_library", "go_test")
+
+go_library(
+    name = "go_default_library",
+    srcs = ["http.go"],
+    importpath = "github.com/uber/submitqueue/platform/errs/http",
+    visibility = ["//visibility:public"],
+    deps = [
+        "//platform/errs:go_default_library",
+        "//platform/http:go_default_library",
+    ],
+)
+
+go_test(
+    name = "go_default_test",
+    srcs = ["http_test.go"],
+    embed = [":go_default_library"],
+    deps = [
+        "//platform/errs:go_default_library",
+        "//platform/errs/generic:go_default_library",
+        "//platform/errs/mysql:go_default_library",
+        "//platform/http:go_default_library",
+        "@com_github_stretchr_testify//assert:go_default_library",
+    ],
+)

platform/errs/http/http.go

@@ -0,0 +1,125 @@
+// Copyright (c) 2025 Uber Technologies, Inc.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+// Package http provides an errs.Classifier for failures returned by HTTP
+// clients: a rejected status code (platform/http.StatusError) and a transport
+// failure (*url.Error, which is what http.Client.Do returns).
+//
+// Wire it into any service whose extensions call an HTTP API — build runners,
+// CI gateways, webhook senders. Without it every status code looks the same to
+// the pipeline: unclassified, and therefore non-retryable, so a 502 from a proxy
+// dead-letters the message on its first attempt rather than being retried.
+//
+// Order matters when wiring this alongside platform/errs/mysql. The MySQL
+// classifier treats any net.Error as retryable infra, and *url.Error satisfies
+// net.Error, so it will claim HTTP transport failures if it runs first. List
+// this classifier before it to keep those failures attributed to the dependency
+// they came from:
+//
+//	errs.NewClassifierProcessor(
+//	    genericerrs.Classifier,
+//	    httperrs.Classifier,
+//	    mysqlerrs.Classifier,
+//	)
+package http
+
+import (
+	"context"
+	nethttp "net/http"
+	"net/url"
+
+	"github.com/uber/submitqueue/platform/errs"
+	phttp "github.com/uber/submitqueue/platform/http"
+)
+
+// Classifier implements errs.Classifier for HTTP client failures. It recognises:
+//
+//   - *phttp.StatusError — dispatches on the status code. Codes that describe a
+//     server-side or overload condition (500, 502, 503, 504, other unassigned
+//     5xx, 429, 408) are retryable dependency errors. Codes that describe a
+//     verdict on the request itself (4xx, 3xx, and the permanently broken 501
+//     and 505) are non-retryable dependency errors.
+//   - *url.Error — the wrapper http.Client.Do puts around connection resets, DNS
+//     failures, TLS errors and timeouts. A retryable dependency error, except
+//     for our own context cancellation (see Classify).
+//
+// Everything else returns errs.Unknown so the classifier-processor walk can keep
+// looking down the unwrap chain.
+//
+// The classifier never returns errs.User. A 400 or 403 says the request was
+// rejected, not that a person did something wrong; only the controller knows
+// whether the request was built from user input. Controllers express that by
+// wrapping with errs.NewUserError, which short-circuits pass 1 of the
+// classifier-processor before this classifier is consulted.
+//
+// The classifier is stateless; this package-level singleton is the canonical
+// handle. Pass it as one of the variadic classifiers to
+// errs.NewClassifierProcessor; the resulting processor is what gets handed to
+// consumer.New.
+var Classifier errs.Classifier = classifier{}
+
+type classifier struct{}
+
+// Classify inspects a single node. Per the errs.Classifier contract, this must
+// not call errors.Is / errors.As — the classifier-processor owns the chain walk.
+func (classifier) Classify(err error) errs.Verdict {
+	if se, ok := err.(*phttp.StatusError); ok {
+		return classifyStatusCode(se.StatusCode)
+	}
+
+	if ue, ok := err.(*url.Error); ok {
+		// A cancelled context is ours, not theirs — process shutdown, or a parent
+		// operation that went away — so decline it and let the generic classifier
+		// claim context.Canceled as plain retryable infra, keeping shutdowns out
+		// of this backend's dependency metrics. An expired deadline is theirs:
+		// the remote end did not answer in time, so it takes the verdict below.
+		// Declining that one would strand it, since generic matches only Canceled.
+		if ue.Err == context.Canceled {
+			return errs.Unknown
+		}
+		// Everything else at this layer is a failed exchange with the remote end,
+		// and none of those shapes says the request was invalid.
+		return errs.InfraDependencyRetryable
+	}
+
+	return errs.Unknown
+}
+
+// classifyStatusCode maps an HTTP status code to a Verdict. The split is whether
+// the code describes the state of the server, which can change on its own, or a
+// verdict on the request, which replaying only reproduces.
+func classifyStatusCode(code int) errs.Verdict {
+	switch code {
+	case nethttp.StatusRequestTimeout, // 408 — the server stopped waiting; sending it again is reasonable.
+		nethttp.StatusTooManyRequests: // 429 — over a rate limit that resets with time.
+		return errs.InfraDependencyRetryable
+
+	case nethttp.StatusNotImplemented, // 501 — the route will not appear because we retried.
+		nethttp.StatusHTTPVersionNotSupported: // 505 — a client/server mismatch to fix in config.
+		return errs.InfraDependency
+	}
+
+	// Remaining 5xx: the server reported its own failure. Covers 500, 502, 503
+	// and 504, the shapes a proxy or overloaded backend produces, plus any
+	// unassigned or vendor-specific 5xx, which follow the same convention.
+	if code >= nethttp.StatusInternalServerError {
+		return errs.InfraDependencyRetryable
+	}
+
+	// 4xx other than the two above, 3xx the client was not configured to follow,
+	// and anything else a caller chose to reject — including a code that was
+	// never a response, such as 0: a verdict on the request, or on a malformed
+	// call. Neither changes on a second attempt.
+	return errs.InfraDependency
+}

platform/errs/http/http_test.go

@@ -0,0 +1,202 @@
+// Copyright (c) 2025 Uber Technologies, Inc.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package http
+
+import (
+	"context"
+	"errors"
+	"fmt"
+	"net"
+	nethttp "net/http"
+	"net/url"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/uber/submitqueue/platform/errs"
+	genericerrs "github.com/uber/submitqueue/platform/errs/generic"
+	mysqlerrs "github.com/uber/submitqueue/platform/errs/mysql"
+	phttp "github.com/uber/submitqueue/platform/http"
+)
+
+func TestClassifier_StatusCodes(t *testing.T) {
+	tests := []struct {
+		name string
+		code int
+		want errs.Verdict
+	}{
+		// Server state: changes without us doing anything differently.
+		{"bad gateway", nethttp.StatusBadGateway, errs.InfraDependencyRetryable},
+		{"service unavailable", nethttp.StatusServiceUnavailable, errs.InfraDependencyRetryable},
+		{"gateway timeout", nethttp.StatusGatewayTimeout, errs.InfraDependencyRetryable},
+		{"internal server error", nethttp.StatusInternalServerError, errs.InfraDependencyRetryable},
+		{"unassigned 5xx", 599, errs.InfraDependencyRetryable},
+		{"request timeout", nethttp.StatusRequestTimeout, errs.InfraDependencyRetryable},
+		{"too many requests", nethttp.StatusTooManyRequests, errs.InfraDependencyRetryable},
+
+		// Verdicts on the request: replaying reproduces the same answer.
+		{"not implemented", nethttp.StatusNotImplemented, errs.InfraDependency},
+		{"http version not supported", nethttp.StatusHTTPVersionNotSupported, errs.InfraDependency},
+		{"bad request", nethttp.StatusBadRequest, errs.InfraDependency},
+		{"unauthorized", nethttp.StatusUnauthorized, errs.InfraDependency},
+		{"forbidden", nethttp.StatusForbidden, errs.InfraDependency},
+		{"not found", nethttp.StatusNotFound, errs.InfraDependency},
+		{"unprocessable entity", nethttp.StatusUnprocessableEntity, errs.InfraDependency},
+		{"unfollowed redirect", nethttp.StatusFound, errs.InfraDependency},
+
+		// Never a response: a caller built this from something else.
+		{"zero code", 0, errs.InfraDependency},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			assert.Equal(t, tt.want, Classifier.Classify(phttp.NewStatusError(tt.code, nil)))
+		})
+	}
+}
+
+func TestClassifier_TransportFailures(t *testing.T) {
+	tests := []struct {
+		name string
+		err  error
+		want errs.Verdict
+	}{
+		{
+			name: "connection refused",
+			err:  &url.Error{Op: "Get", URL: "http://api.example", Err: errors.New("connection refused")},
+			want: errs.InfraDependencyRetryable,
+		},
+		{
+			name: "dns failure",
+			err:  &url.Error{Op: "Get", URL: "http://api.example", Err: &net.DNSError{Err: "no such host"}},
+			want: errs.InfraDependencyRetryable,
+		},
+		{
+			// Ours, not theirs: declining lets the walk reach context.Canceled,
+			// where the generic classifier calls it plain retryable infra.
+			name: "context cancelled",
+			err:  &url.Error{Op: "Get", URL: "http://api.example", Err: context.Canceled},
+			want: errs.Unknown,
+		},
+		{
+			// Theirs, not ours: the remote end did not answer in time.
+			name: "context deadline exceeded",
+			err:  &url.Error{Op: "Get", URL: "http://api.example", Err: context.DeadlineExceeded},
+			want: errs.InfraDependencyRetryable,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			assert.Equal(t, tt.want, Classifier.Classify(tt.err))
+		})
+	}
+}
+
+func TestClassifier_Unknown(t *testing.T) {
+	tests := []struct {
+		name string
+		err  error
+	}{
+		// Per-node contract: a wrapped StatusError must not match here. The
+		// classifier-processor walk reaches the inner node and asks again there.
+		{"wrapped status error", fmt.Errorf("get build x: %w", phttp.NewStatusError(502, nil))},
+		{"plain error", errors.New("anything")},
+		{"bare context.Canceled", context.Canceled},
+		{"nil", nil},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			assert.Equal(t, errs.Unknown, Classifier.Classify(tt.err))
+		})
+	}
+}
+
+func TestClassifier_AppliedViaProcessor(t *testing.T) {
+	// The order services wire: generic first, this one before mysqlerrs.
+	processor := errs.NewClassifierProcessor(genericerrs.Classifier, Classifier, mysqlerrs.Classifier)
+
+	t.Run("wrapped 502 becomes a retryable dependency error", func(t *testing.T) {
+		err := fmt.Errorf("get build org/pipeline/builds/1: %w", phttp.NewStatusError(nethttp.StatusBadGateway, []byte("proxy forward failed")))
+		out := processor.Process(err)
+		assert.True(t, errs.IsRetryable(out))
+		assert.True(t, errs.IsDependencyError(out))
+	})
+
+	t.Run("wrapped 404 stays non-retryable", func(t *testing.T) {
+		err := fmt.Errorf("get build org/pipeline/builds/1: %w", phttp.NewStatusError(nethttp.StatusNotFound, nil))
+		out := processor.Process(err)
+		assert.False(t, errs.IsRetryable(out))
+		assert.True(t, errs.IsDependencyError(out))
+	})
+
+	t.Run("transport failure is attributed to the dependency not mysql", func(t *testing.T) {
+		// mysqlerrs calls any net.Error retryable infra, and *url.Error is one,
+		// so it would claim this node and drop the dependency attribution if it
+		// were listed first.
+		err := fmt.Errorf("send: %w", &url.Error{Op: "Get", URL: "http://api.example", Err: errors.New("connection reset by peer")})
+		out := processor.Process(err)
+		assert.True(t, errs.IsRetryable(out))
+		assert.True(t, errs.IsDependencyError(out), "should be attributed to the HTTP dependency")
+	})
+
+	t.Run("our cancellation is retryable but not a dependency failure", func(t *testing.T) {
+		err := fmt.Errorf("send: %w", &url.Error{Op: "Get", URL: "http://api.example", Err: context.Canceled})
+		out := processor.Process(err)
+		assert.True(t, errs.IsRetryable(out))
+		assert.False(t, errs.IsDependencyError(out))
+	})
+
+	t.Run("expired deadline is retryable without mysqlerrs claiming it", func(t *testing.T) {
+		err := fmt.Errorf("send: %w", &url.Error{Op: "Get", URL: "http://api.example", Err: context.DeadlineExceeded})
+		out := processor.Process(err)
+		assert.True(t, errs.IsRetryable(out))
+		assert.True(t, errs.IsDependencyError(out), "should be attributed to the HTTP dependency")
+	})
+
+	t.Run("a controller verdict wins over the classifier", func(t *testing.T) {
+		// Pass 1 of the processor short-circuits on the existing framework wrap,
+		// so a 502 a controller decided was fatal stays fatal.
+		err := errs.NewDependencyError(phttp.NewStatusError(nethttp.StatusBadGateway, nil))
+		out := processor.Process(err)
+		assert.Same(t, err, out)
+		assert.False(t, errs.IsRetryable(out))
+	})
+}
+
+// TestClassifier_WithoutMySQLClassifier covers a service with no MySQL
+// dependency: no verdict here may rely on mysqlerrs' net.Error rule.
+func TestClassifier_WithoutMySQLClassifier(t *testing.T) {
+	processor := errs.NewClassifierProcessor(genericerrs.Classifier, Classifier)
+
+	tests := []struct {
+		name           string
+		cause          error
+		wantDependency bool
+	}{
+		{name: "connection reset", cause: errors.New("connection reset by peer"), wantDependency: true},
+		{name: "expired deadline", cause: context.DeadlineExceeded, wantDependency: true},
+		{name: "our cancellation", cause: context.Canceled, wantDependency: false},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			err := fmt.Errorf("send: %w", &url.Error{Op: "Get", URL: "http://api.example", Err: tt.cause})
+			out := processor.Process(err)
+			assert.True(t, errs.IsRetryable(out), "must not depend on mysqlerrs being wired")
+			assert.Equal(t, tt.wantDependency, errs.IsDependencyError(out))
+		})
+	}
+}