Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion platform/errs/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:

Expand Down Expand Up @@ -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
Expand Down
25 changes: 25 additions & 0 deletions platform/errs/http/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -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",
],
)
125 changes: 125 additions & 0 deletions platform/errs/http/http.go
Original file line number Diff line number Diff line change
@@ -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
Comment thread
mnoah1 marked this conversation as resolved.
}
202 changes: 202 additions & 0 deletions platform/errs/http/http_test.go
Original file line number Diff line number Diff line change
@@ -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))
})
}
}
Loading
Loading