Skip to content

Commit aa7ef40

Browse files
committed
feat(messagequeue)!: carry a structured failure across the dead-letter boundary
## Summary ### Why? Today the only thing that crosses the dead-letter boundary is a string, and on the most common path not even that. `Nack(ctx)` takes no reason at all, so when a retryable error exhausts its budget the poll loop dead-letters the message with the hardcoded literal `"exceeded retry limit"`. The error that actually caused it exists only in a log line. `Reject` does better — it passes `err.Error()` — but that is a flattened `fmt.Errorf` chain, so a consumer wanting to know *what* failed has nothing to read but prose. That is a problem for any stage whose work is wider than the message that triggers it. Such a stage can fail because of an entity the message does not name, or because of nothing in particular, and a dead-letter reconciler has no way to tell — so it acts on the entity it does have and can terminate the wrong one. ### What? A failure now travels as data. `platform/base/failure` defines `Failure{Message, Subjects, Detail}`, where a `Subject{Type, ID}` names an entity the failure is about. Types are domain-chosen, so the platform stays domain-agnostic. A failure is always about *something*: when no single record is at fault the subject is the wider thing that is, which leaves an empty subject list to mean "unattributed" rather than "nothing was to blame". The pieces: - `errs.Attribute` / `errs.Detail` attach subjects and context to an error; `errs.Attribution` reads them back, merging across layers. The wrapper implements `Unwrap` and is neither a `userError` nor an `infraError`, so classification walks straight through it and retryability is unchanged. - `Nack` and `Reject` take a `Failure`, and `Delivery.Failure()` returns the one recorded against a dead-lettered message. - The consumer builds the failure from the error the controller returned, defaulting the message to `err.Error()`. A controller that attributes nothing produces exactly what callers sent before, which is what makes this change behaviour-neutral. - `Nack` also dead-letters directly once the retry budget is spent, so the attempt still holding the reason is the one that records it. The poll-time check stays as a backstop for a delivery that never reaches `Nack` — a crash, or a visibility timeout — and that path keeps the generic literal. Message count is unchanged: at attempt N the next poll would have dead-lettered iff `N >= MaxAttempts`, which is the condition `Nack` now applies. Storage splits the failure across two columns of `queue_messages`. `last_error` keeps the human-readable message, unchanged in meaning, so nothing has to decode it and `SELECT last_error` stays useful. A new `failure_detail JSON` column holds subjects and detail. The split is what makes the round trip unambiguous — one column would force a decoder to guess whether the text is an envelope or a message, and any error whose text happened to be valid JSON would decode as a malformed envelope and lose the message. An absent `failure_detail` means unattributed, with no heuristic involved, which is exactly the state of rows written before this column existed and of the retry-limit backstop. `failure_detail` is nullable rather than taking an empty sentinel like its neighbours: a JSON column rejects `''`. `MoveToDLQ` binds SQL NULL explicitly instead of a nil `[]byte`, so the value does not depend on driver conversion. No behaviour changes for any existing caller. This is the mechanism a follow-up uses to attribute speculation failures and stop them stranding a queue. ## Test Plan ✅ `bazel test //platform/... //submitqueue/... //runway/... //stovepipe/...` ✅ `bazel test //test/integration/extension/messagequeue/...` ✅ `make lint check-tidy check-gazelle check-mocks` New coverage: - `platform/base/failure` — codec round trip including nested detail, and the `float64` number-decoding behaviour pinned so it cannot surprise a caller later. - `platform/errs` — the regression that matters: `errors.As` still reaches a wrapped driver error *through* the envelope, and a classifier still marks it retryable. Had the wrapper broken the chain walk, every storage error would have silently stopped being retryable. - `platform/extension/messagequeue/mysql` — the nack-time dead-letter boundary as a table (budget remaining, one attempt left, final attempt, single-attempt budget, unset budget), because dead-lettering one attempt early would silently cost every message a retry. - `platform/consumer` — an unattributed controller error yields a message-only failure, proving behaviour-neutrality. - Integration — the DLQ test previously asserted `dlq.last_error == "exceeded retry limit"`; it now asserts the real reason from the final nack, and that the subject survives the round trip. ## Issue Part of CODEM-428
1 parent 4ce8e54 commit aa7ef40

26 files changed

Lines changed: 1030 additions & 81 deletions

platform/base/failure/BUILD.bazel

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
load("@rules_go//go:def.bzl", "go_library", "go_test")
2+
3+
go_library(
4+
name = "go_default_library",
5+
srcs = ["failure.go"],
6+
importpath = "github.com/uber/submitqueue/platform/base/failure",
7+
visibility = ["//visibility:public"],
8+
)
9+
10+
go_test(
11+
name = "go_default_test",
12+
srcs = ["failure_test.go"],
13+
embed = [":go_default_library"],
14+
deps = [
15+
"@com_github_stretchr_testify//assert:go_default_library",
16+
"@com_github_stretchr_testify//require:go_default_library",
17+
],
18+
)

platform/base/failure/failure.go

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
// Copyright (c) 2025 Uber Technologies, Inc.
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
// Package failure holds the shared description of why processing failed: a
16+
// human-readable message, the entities the failure is about, and free-form
17+
// detail. It is the vocabulary a producer of a failure and a consumer of it
18+
// share when they are separated by a queue, so the consumer reads fields
19+
// rather than parsing prose.
20+
//
21+
// The package is deliberately domain-agnostic. It says a failure has subjects
22+
// and what shape a subject is; which subject types exist is a domain's own
23+
// business.
24+
package failure
25+
26+
import "encoding/json"
27+
28+
// Subject names one entity a failure is about.
29+
//
30+
// Its purpose is attribution: a consumer reconciling a failure has to know
31+
// what to act on, and the entity named on the message that failed is not
32+
// always the entity at fault — a job that reads many records can fail because
33+
// of any of them, or because of none of them individually.
34+
type Subject struct {
35+
// Type labels what kind of entity ID names, e.g. "batch" or "queue".
36+
// Values are chosen by the domain that raises the failure; this package
37+
// neither defines nor validates them. Empty means the type is unknown.
38+
Type string `json:"type"`
39+
// ID identifies the entity within its type. Opaque here: no format is
40+
// assumed and none is parsed.
41+
ID string `json:"id"`
42+
}
43+
44+
// Failure describes why processing failed.
45+
//
46+
// A failure is always about something. When no single record is at fault, the
47+
// subject is the wider thing that is — the queue, the tenant, the job — rather
48+
// than an empty list. That keeps absence from carrying meaning: no subjects at
49+
// all means the failure is *unattributed*, which is a genuine third state
50+
// (nothing recorded one, or the record predates attribution) and not a claim
51+
// that nothing was to blame.
52+
type Failure struct {
53+
// Message is the human-readable reason, typically an error's text. It is
54+
// the one field always present, and the one a person reads first.
55+
Message string `json:"-"`
56+
// Subjects are the entities this failure is about, in no significant
57+
// order. Empty means unattributed — see the type comment.
58+
Subjects []Subject `json:"subjects,omitempty"`
59+
// Detail is free-form structured context: whatever the producer knows that
60+
// does not fit the message. Values survive a JSON round trip, so numbers
61+
// come back as float64 regardless of what went in.
62+
Detail map[string]any `json:"detail,omitempty"`
63+
}
64+
65+
// New builds a Failure with a message and the subjects it is about.
66+
func New(message string, subjects ...Subject) Failure {
67+
return Failure{Message: message, Subjects: subjects}
68+
}
69+
70+
// IDsOfType returns the IDs of every subject with the given type, in the order
71+
// they appear. The result is empty when the failure names no such subject,
72+
// which is how a consumer asks "is this about one of mine?" without inspecting
73+
// the slice itself.
74+
func (f Failure) IDsOfType(subjectType string) []string {
75+
var ids []string
76+
for _, s := range f.Subjects {
77+
if s.Type == subjectType {
78+
ids = append(ids, s.ID)
79+
}
80+
}
81+
return ids
82+
}
83+
84+
// Encode returns the JSON encoding of the structured half of f — its subjects
85+
// and detail — or nil when there is no structure to store.
86+
//
87+
// Message is deliberately excluded. It travels as plain text alongside this
88+
// blob so that it stays legible to anything reading the underlying store
89+
// directly, and so decoding never has to guess whether a stored string is an
90+
// encoded failure or a message that merely looks like one.
91+
func Encode(f Failure) ([]byte, error) {
92+
if len(f.Subjects) == 0 && len(f.Detail) == 0 {
93+
return nil, nil
94+
}
95+
return json.Marshal(f)
96+
}
97+
98+
// Decode parses the structured half produced by Encode. Empty input yields the
99+
// zero Failure, which is how an unattributed failure reads.
100+
//
101+
// The returned Message is always empty: the caller holds it separately and
102+
// fills it in.
103+
func Decode(data []byte) (Failure, error) {
104+
if len(data) == 0 {
105+
return Failure{}, nil
106+
}
107+
var f Failure
108+
if err := json.Unmarshal(data, &f); err != nil {
109+
return Failure{}, err
110+
}
111+
return f, nil
112+
}
Lines changed: 127 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,127 @@
1+
// Copyright (c) 2025 Uber Technologies, Inc.
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package failure
16+
17+
import (
18+
"testing"
19+
20+
"github.com/stretchr/testify/assert"
21+
"github.com/stretchr/testify/require"
22+
)
23+
24+
func TestRoundTrip(t *testing.T) {
25+
tests := []struct {
26+
name string
27+
in Failure
28+
want Failure
29+
}{
30+
{
31+
name: "subjects and detail",
32+
in: New("speculator failed", Subject{Type: "queue", ID: "test-queue"}).
33+
withDetail(map[string]any{"stage": "ask"}),
34+
want: Failure{
35+
Subjects: []Subject{{Type: "queue", ID: "test-queue"}},
36+
Detail: map[string]any{"stage": "ask"},
37+
},
38+
},
39+
{
40+
name: "several subjects keep their order",
41+
in: New("two at fault", Subject{Type: "batch", ID: "q/batch/2"}, Subject{Type: "batch", ID: "q/batch/1"}),
42+
want: Failure{Subjects: []Subject{{Type: "batch", ID: "q/batch/2"}, {Type: "batch", ID: "q/batch/1"}}},
43+
},
44+
{
45+
name: "nested detail survives",
46+
in: Failure{Detail: map[string]any{"path": map[string]any{"id": "abc"}}},
47+
want: Failure{Detail: map[string]any{"path": map[string]any{"id": "abc"}}},
48+
},
49+
}
50+
51+
for _, tt := range tests {
52+
t.Run(tt.name, func(t *testing.T) {
53+
encoded, err := Encode(tt.in)
54+
require.NoError(t, err)
55+
require.NotEmpty(t, encoded)
56+
57+
got, err := Decode(encoded)
58+
require.NoError(t, err)
59+
assert.Equal(t, tt.want, got)
60+
})
61+
}
62+
}
63+
64+
// The message is carried outside the blob so that whatever stores it keeps a
65+
// legible column, and so decoding never has to tell an encoded failure apart
66+
// from a message that happens to look like one.
67+
func TestEncodeOmitsMessage(t *testing.T) {
68+
encoded, err := Encode(New("boom", Subject{Type: "batch", ID: "q/batch/1"}))
69+
require.NoError(t, err)
70+
assert.NotContains(t, string(encoded), "boom")
71+
72+
got, err := Decode(encoded)
73+
require.NoError(t, err)
74+
assert.Empty(t, got.Message)
75+
assert.Equal(t, []Subject{{Type: "batch", ID: "q/batch/1"}}, got.Subjects)
76+
}
77+
78+
// Nothing structured means nothing to store, which is what lets a caller treat
79+
// an absent blob as "unattributed" without a sentinel.
80+
func TestEncodeNothingStructured(t *testing.T) {
81+
encoded, err := Encode(New("just a message"))
82+
require.NoError(t, err)
83+
assert.Nil(t, encoded)
84+
}
85+
86+
func TestDecodeEmpty(t *testing.T) {
87+
got, err := Decode(nil)
88+
require.NoError(t, err)
89+
assert.Equal(t, Failure{}, got)
90+
}
91+
92+
func TestDecodeMalformed(t *testing.T) {
93+
_, err := Decode([]byte("not json"))
94+
assert.Error(t, err)
95+
}
96+
97+
// Detail goes through encoding/json, so every number returns as a float64
98+
// whatever its Go type going in. Pinned because a caller that stores an int64
99+
// and reads it back expecting one would otherwise find out at runtime.
100+
func TestDetailNumbersDecodeAsFloat64(t *testing.T) {
101+
encoded, err := Encode(Failure{Detail: map[string]any{"attempt": int64(3)}})
102+
require.NoError(t, err)
103+
104+
got, err := Decode(encoded)
105+
require.NoError(t, err)
106+
assert.Equal(t, float64(3), got.Detail["attempt"])
107+
}
108+
109+
func TestIDsOfType(t *testing.T) {
110+
f := New("mixed",
111+
Subject{Type: "batch", ID: "q/batch/1"},
112+
Subject{Type: "queue", ID: "q"},
113+
Subject{Type: "batch", ID: "q/batch/2"},
114+
)
115+
116+
assert.Equal(t, []string{"q/batch/1", "q/batch/2"}, f.IDsOfType("batch"))
117+
assert.Equal(t, []string{"q"}, f.IDsOfType("queue"))
118+
assert.Empty(t, f.IDsOfType("request"))
119+
assert.Empty(t, Failure{}.IDsOfType("batch"))
120+
}
121+
122+
// withDetail keeps the table above readable; New covers message and subjects,
123+
// which is what most callers set.
124+
func (f Failure) withDetail(detail map[string]any) Failure {
125+
f.Detail = detail
126+
return f
127+
}

platform/consumer/BUILD.bazel

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ go_library(
1010
importpath = "github.com/uber/submitqueue/platform/consumer",
1111
visibility = ["//visibility:public"],
1212
deps = [
13+
"//platform/base/failure:go_default_library",
1314
"//platform/base/messagequeue:go_default_library",
1415
"//platform/errs:go_default_library",
1516
"//platform/extension/consumergate:go_default_library",
@@ -28,6 +29,7 @@ go_test(
2829
],
2930
embed = [":go_default_library"],
3031
deps = [
32+
"//platform/base/failure:go_default_library",
3133
"//platform/base/messagequeue:go_default_library",
3234
"//platform/errs:go_default_library",
3335
"//platform/extension/consumergate:go_default_library",

platform/consumer/consumer.go

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -424,6 +424,12 @@ func (m *consumer) processDelivery(ctx context.Context, controller Controller, d
424424
// cancelled by the processing context during shutdown.
425425
isCanceled := errors.Is(err, context.Canceled)
426426

427+
// Whatever the controller attributed the failure to, plus the error's
428+
// own text as the message. A controller that attributed nothing yields
429+
// the message alone, which is what every caller sent before failures
430+
// carried structure.
431+
controllerFailure := errs.Attribution(err)
432+
427433
// Check if the error is non-retryable (poison pill message)
428434
if !errs.IsRetryable(err) {
429435
m.logger.Errorw("non-retryable controller error, rejecting message",
@@ -438,7 +444,7 @@ func (m *consumer) processDelivery(ctx context.Context, controller Controller, d
438444

439445
// Reject moves to DLQ (or acks if DLQ disabled)
440446
rejectOp := metrics.Begin(controllerScope, "reject", metrics.StorageLatencyBuckets)
441-
rejectErr := delivery.Reject(ctx, err.Error())
447+
rejectErr := delivery.Reject(ctx, controllerFailure)
442448
rejectOp.Complete(rejectErr)
443449
if rejectErr != nil {
444450
m.logger.Errorw("failed to reject non-retryable message",
@@ -468,9 +474,11 @@ func (m *consumer) processDelivery(ctx context.Context, controller Controller, d
468474
"elapsed_ms", elapsed.Milliseconds(),
469475
)
470476

471-
// Nack requeues immediately - the visibility timeout spaces retries
477+
// Nack requeues immediately - the visibility timeout spaces retries.
478+
// The failure travels with it so that the attempt which finally spends
479+
// the retry budget can dead-letter saying why.
472480
nackOp := metrics.Begin(controllerScope, "nack", metrics.StorageLatencyBuckets)
473-
nackErr := delivery.Nack(ctx)
481+
nackErr := delivery.Nack(ctx, controllerFailure)
474482
nackOp.Complete(nackErr)
475483
if nackErr != nil {
476484
m.logger.Errorw("failed to nack message",

0 commit comments

Comments
 (0)