Nobody has started this yet — be first.
Business impact
This is, arguably, the single worst possible defect a retry-and-DLQ library aimed at payments-style reliability can have: under a specific, plausible misconfiguration, an operation silently never runs at all, and the caller is told it succeeded. A Policy{} zero value reaches Do/DoWithResult any time a caller constructs a policy from partially-populated config and forgets (or a bug elsewhere fails) to set MaxAttempts -- and instead of falling back to a sane default (as every other field already does), the operation is skipped entirely and reported as a clean success. For a charge, that's "the customer was told their payment went through and it never actually touched the payment processor" -- silent, undetectable by the caller, and it will never appear in the DLQ either, because nothing failed. This is precisely the "No silent data loss" guarantee the project's own README explicitly calls out, defeated by a single flipped comparison operator.
Problem
Policy.WithDefaults is supposed to normalize any non-positive MaxAttempts (including the zero value, i.e. "caller didn't set it") up to DefaultPolicy's value of 5. It only normalizes strictly negative values.
Current behavior
retry.Do(ctx, fn, retry.Policy{}) returns nil (success) without ever calling fn.
Expected behavior
retry.Do(ctx, fn, retry.Policy{}) should behave exactly like retry.Do(ctx, fn, retry.DefaultPolicy()) -- fn is called (up to 5 times, per DefaultPolicy), and the returned error (or lack of one) reflects what fn actually did.
Steps to reproduce
called := false err := retry.Do(context.Background(), func(ctx context.Context) error { called = true return errors.New("boom") }, retry.Policy{}) // zero value: MaxAttempts unset fmt.Println(called, err) // got: false <nil>, want: true <some error>
Why this matters
Every other field on Policy correctly falls back to its DefaultPolicy value when left unset (BaseDelay <= 0, MaxDelay <= 0, etc. -- all use <= 0, not < 0). MaxAttempts is the one outlier, and it happens to be the field whose zero value has the most catastrophic possible consequence: DoWithResult's attempt loop is a plain for attempt := 1; attempt <= policy.MaxAttempts; attempt++, so MaxAttempts == 0 makes the loop body never execute even once, leaving the zero-value Result{} (whose Err field defaults to nil) as the entire return value -- a false success indistinguishable, from the caller's point of view, from a real one.
Suggested approach
Every other normalization check in WithDefaults (pkg/retry/policy.go) uses the same comparison operator against zero. Find the one that doesn't match its siblings.
Acceptance criteria
Verification
go test ./test/ -run TestTask12 -v
Hints (0/2)
Try it without hints first — the reading is the exercise.
Working on this ticket
Work on a branch named for the ticket — that's what you'll submit.
Branch off your fork
Sign in to create it automatically, or do it yourself:
$git checkout -b fix/zero-max-attempts-silent-successFix it and commit
Meet every acceptance criterion, and add a test that would have caught this.
Push the branch
$git push -u origin fix/zero-max-attempts-silent-successSubmit it below
Paste your fork URL and the branch name, with a short write-up of the root cause.
Questions
Ask about anything unclear in the ticket — the maintainer and anyone who has solved it can answer. Please don't post full solutions.
Sign in to ask a question or reply.
Sign inhttp://localhost:8080POST /v1/chargesTo run against Redis (the multi-replica-safe backend) with zero local setup:
docker compose up --build # app + redis, hot-reloading on save
Work the tickets in tasks/ (ordered easy → hard); each names one Go test in test/:
make test-race # go test -race -count=1 ./...
go test ./test/ -run TestTask01 -v # a single ticket
Level 1
Fix a bug
Read existing behaviour, correct it.