Skip to content

Commit 1885834

Browse files
committed
backup: #904 v10 - classify adapter-data rejections as exit 2
## Codex P2 v9: adapter encoder rejections fall through to exit 1 When an adapter encoder rejects the input tree's contents (a malformed DynamoDB _schema.json, an S3 collision artifact the encoder cannot reverse, an SQS side-record with an unknown kind, etc.), the error propagates out of EncodeSnapshot but does not match any of the three exit-2 sentinels (ErrSelfTestLowerLastCommitTS, ErrEncodeUnsupportedDynamoDBLayout, errSelfTestMismatch), so the CLI's run() falls through to exit 1. Per the CLI contract, exit 1 is operator/flag error and exit 2 is data-correctness — runbooks branch on exit status to quarantine bad dump data, so misclassifying adapter rejections as user error is a real ergonomic regression. The encoder is offline-only — it operates entirely on local files under InputRoot. Every error from an adapter encoder originates from the contents of that tree (a sentinel rejection, an unmarshalling failure, etc.). Treating all adapter errors as data-correctness matches the CLI contract. Fix: - New ErrEncodeAdapterData sentinel in internal/backup. - runAdapterEncoders wraps each adapter error with errors.Mark so errors.Is(err, ErrEncodeAdapterData) is true. Mark preserves the inner sentinel chain — callers that errors.Is on the per-adapter sentinel (ErrDDBEncodeInvalidSchema, ErrS3EncodeKeyConflict, etc.) are unaffected. - CLI run() gains a fourth exit-2 branch for ErrEncodeAdapterData. Pinned by: - TestEncodeSnapshotMarksAdapterDataErrors (library): malformed _schema.json triggers ErrDDBEncodeInvalidSchema inside the DDB encoder; assertion verifies BOTH errors.Is(ErrEncodeAdapterData) AND errors.Is(ErrDDBEncodeInvalidSchema) hold - the mark is additive, not lossy. - TestCLIAdapterDataErrorExitsTwo (CLI): same fixture driven end-to-end through run(); asserts exit code = 2 and no .fsm published. ## Caller audit per CLAUDE.md semantic-change rule - runAdapterEncoders return semantics: every non-nil err is now marked. Sole caller is EncodeSnapshot; it propagates the marked error unchanged. - EncodeSnapshot callers (library tests, CLI's encodeToTempFile): - Library tests that errors.Is on the validation sentinels fire BEFORE runAdapterEncoders, so they are NOT marked. - CLI's encodeToTempFile wraps with "EncodeSnapshot"; the run() layer above it now matches ErrEncodeAdapterData and routes to exit-2 (new branch). - In-tree errors.Is on per-adapter sentinels (encode_dynamodb_test.go et al.) call the adapter encoders directly, not through EncodeSnapshot, so the mark wrapper is never on their path. Tests + lint green. wrapcheck required errors.WithStack around the Mark.
1 parent 145a912 commit 1885834

4 files changed

Lines changed: 106 additions & 3 deletions

File tree

cmd/elastickv-snapshot-encode/main.go

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -82,14 +82,20 @@ func run(argv []string, logger *slog.Logger) (int, error) {
8282
}
8383
if err := encodeOne(cfg, logger); err != nil {
8484
// Errors from the encoder layer that represent data constraints
85-
// (HLC ceiling regression, self-test mismatch) are exit 2; other
86-
// errors are exit 1.
85+
// (HLC ceiling regression, JSONL layout, self-test mismatch,
86+
// adapter rejecting input-tree contents) are exit 2; flag/path
87+
// errors are exit 1. Runbooks branch on exit status to triage
88+
// bad-dump-data vs operator typos, so this split is part of the
89+
// CLI contract (codex P2 v9 #904 added the adapter-data branch).
8790
if errors.Is(err, backup.ErrSelfTestLowerLastCommitTS) {
8891
return exitDataErr, err
8992
}
9093
if errors.Is(err, backup.ErrEncodeUnsupportedDynamoDBLayout) {
9194
return exitDataErr, err
9295
}
96+
if errors.Is(err, backup.ErrEncodeAdapterData) {
97+
return exitDataErr, err
98+
}
9399
if errors.Is(err, errSelfTestMismatch) {
94100
return exitDataErr, err
95101
}

cmd/elastickv-snapshot-encode/main_test.go

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,46 @@ func TestCLIRejectsUnknownAdapter(t *testing.T) {
7777
}
7878
}
7979

80+
// TestCLIAdapterDataErrorExitsTwo pins codex P2 v9 #904: when an
81+
// adapter encoder rejects the input tree's contents (e.g. a malformed
82+
// DynamoDB _schema.json with an empty table_name), the CLI exits 2
83+
// (data-correctness) rather than 1 (operator/flag error) so runbooks
84+
// can branch on exit status to quarantine bad dump data. Pinned via
85+
// the same ErrDDBEncodeInvalidSchema fixture pattern used by the
86+
// in-package DynamoDB encoder tests.
87+
func TestCLIAdapterDataErrorExitsTwo(t *testing.T) {
88+
t.Parallel()
89+
in := t.TempDir()
90+
emitMinimalManifest(t, in, 100)
91+
// Empty table_name triggers ErrDDBEncodeInvalidSchema inside the
92+
// DynamoDB encoder; runAdapterEncoders marks it with
93+
// ErrEncodeAdapterData; run() maps that to exitDataErr.
94+
schemaDir := filepath.Join(in, "dynamodb", "tbl")
95+
if err := os.MkdirAll(schemaDir, 0o755); err != nil {
96+
t.Fatalf("MkdirAll: %v", err)
97+
}
98+
schemaPath := filepath.Join(schemaDir, "_schema.json")
99+
body := []byte(`{"format_version":1,"table_name":"","primary_key":{"hash_key":{"name":"id","type":"S"}}}`)
100+
if err := os.WriteFile(schemaPath, body, 0o600); err != nil {
101+
t.Fatalf("WriteFile: %v", err)
102+
}
103+
out := filepath.Join(t.TempDir(), "out.fsm")
104+
code, err := run([]string{
105+
"--input", in,
106+
"--output", out,
107+
"--adapter", "dynamodb",
108+
}, quietLogger())
109+
if err == nil {
110+
t.Fatalf("run succeeded; want adapter rejection error")
111+
}
112+
if code != exitDataErr {
113+
t.Errorf("exit code = %d, want %d (data error from adapter rejection, not flag-parse error)", code, exitDataErr)
114+
}
115+
if _, statErr := os.Stat(out); !os.IsNotExist(statErr) {
116+
t.Errorf(".fsm exists at %s; should not be published on adapter rejection", out)
117+
}
118+
}
119+
80120
// TestCLIRejectsLowerLastCommitTSOverride is the fail-closed pin per
81121
// parent §"MVCC re-encoding": T < manifest.last_commit_ts → exit 2
82122
// (data-correctness failure, not flag-parse error).

internal/backup/encode_snapshot.go

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,22 @@ var ErrSelfTestLowerLastCommitTS = errors.New("backup: --last-commit-ts T < mani
4141
// until the encoder learns the JSONL layout (M7 / future milestone).
4242
var ErrEncodeUnsupportedDynamoDBLayout = errors.New("backup: DynamoDB JSONL layout not supported by encoder")
4343

44+
// ErrEncodeAdapterData marks every error returned by an adapter
45+
// encoder (Redis / DynamoDB / S3 / SQS) so callers can distinguish
46+
// "the input tree contained content the encoder cannot translate"
47+
// from "operator passed a bad flag". The encoder is offline-only —
48+
// every adapter error originates from rejecting the content under
49+
// opts.InputRoot (a malformed DynamoDB _schema.json, an S3 collision
50+
// artifact the encoder cannot reverse, a SQS side-record with an
51+
// unknown kind, …). These are data-correctness failures, not user
52+
// errors; the CLI maps this sentinel to exit 2 so runbooks can branch
53+
// on exit status to quarantine bad dump data (codex P2 v9 #904).
54+
//
55+
// Wrapped via errors.Mark inside runAdapterEncoders so the original
56+
// adapter sentinel chain (ErrDDBEncodeInvalidSchema, …) is preserved
57+
// for callers that errors.Is on the more specific type.
58+
var ErrEncodeAdapterData = errors.New("backup: adapter encoder rejected input tree")
59+
4460
// The encoder dispatch order (redis → dynamodb → s3 → sqs) is encoded
4561
// inside adapterRunners() and is intentionally distinct from decode.go's
4662
// finalize order (dynamodb → s3 → redis → sqs). The final .fsm byte
@@ -353,14 +369,20 @@ func adapterRunners() []adapterRunner {
353369
// runAdapterEncoders invokes each enabled adapter encoder in
354370
// canonicalAdapterFanOutOrder, returning the list of adapter names
355371
// actually invoked (for ENCODE_INFO.json adapters_enabled).
372+
//
373+
// Adapter errors are marked with ErrEncodeAdapterData so the CLI can
374+
// route them to exit-2 (data-correctness) rather than exit-1 (user
375+
// error). The original adapter sentinel chain is preserved — callers
376+
// that errors.Is on ErrDDBEncodeInvalidSchema, ErrS3EncodeKeyConflict,
377+
// etc. still see those (codex P2 v9 #904).
356378
func runAdapterEncoders(b *snapshotBuilder, opts EncodeOptions) ([]string, error) {
357379
var enabled []string
358380
for _, r := range adapterRunners() {
359381
if !r.enabled(opts.Adapters) {
360382
continue
361383
}
362384
if err := r.encode(b, opts.InputRoot); err != nil {
363-
return nil, err
385+
return nil, errors.WithStack(errors.Mark(err, ErrEncodeAdapterData))
364386
}
365387
enabled = append(enabled, r.name)
366388
}

internal/backup/encode_snapshot_test.go

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -388,6 +388,41 @@ func TestEncodeSnapshotRejectsZeroAdapterSet(t *testing.T) {
388388
}
389389
}
390390

391+
// TestEncodeSnapshotMarksAdapterDataErrors pins codex P2 v9 #904: when
392+
// an adapter encoder rejects the input tree's contents (e.g. a
393+
// malformed DynamoDB _schema.json), EncodeSnapshot must surface the
394+
// failure as ErrEncodeAdapterData so the CLI can route it to exit-2
395+
// (data-correctness) rather than exit-1 (operator/flag error).
396+
// Crucially, errors.Mark preserves the original sentinel chain, so a
397+
// caller that errors.Is on the per-adapter sentinel
398+
// (ErrDDBEncodeInvalidSchema here) still gets a match — the marking
399+
// is additive.
400+
func TestEncodeSnapshotMarksAdapterDataErrors(t *testing.T) {
401+
t.Parallel()
402+
in := t.TempDir()
403+
// Empty table_name triggers ErrDDBEncodeInvalidSchema inside the
404+
// DynamoDB encoder (encode_dynamodb.go:120).
405+
writeDDBSchema(t, in, "tbl",
406+
[]byte(`{"format_version":1,"table_name":"","primary_key":{"hash_key":{"name":"id","type":"S"}}}`))
407+
var buf bytes.Buffer
408+
_, err := EncodeSnapshot(EncodeOptions{
409+
InputRoot: in,
410+
Adapters: AdapterSet{DynamoDB: true},
411+
LastCommitTS: 1,
412+
}, &buf)
413+
if err == nil {
414+
t.Fatalf("EncodeSnapshot with malformed schema succeeded; want error")
415+
}
416+
if !errors.Is(err, ErrEncodeAdapterData) {
417+
t.Errorf("err = %v, want errors.Is ErrEncodeAdapterData", err)
418+
}
419+
// Inner sentinel must still be reachable so existing per-adapter
420+
// errors.Is callers are unaffected by the additional mark.
421+
if !errors.Is(err, ErrDDBEncodeInvalidSchema) {
422+
t.Errorf("err = %v, want errors.Is ErrDDBEncodeInvalidSchema (mark must preserve inner chain)", err)
423+
}
424+
}
425+
391426
// TestEncodeInfoSidecarPath pins the path-derivation rule for the
392427
// sidecar (gemini medium v2 #896): one .fsm path produces one distinct
393428
// sidecar path; two .fsm files in the same dir produce two distinct

0 commit comments

Comments
 (0)