Make base error classification method-aware so non-idempotent calls are not re-sent

Aperta
  • Qualcosa non va?
#172 0 commenti 0 reazioni 1 assegnatario Vedi su GitHub

I maintainer di solito rispondono entro 1 giorno

@ffumero2003 ci sta già lavorando.

Dal 8/11/2026.

Valutazione

Difficoltà
4/5
Tempo stimato
3-5 giorni
Idoneità per principianti
45/100
Tipo di issue
Bug
Chiarezza
Specificata chiaramente
Stato di attività
Attiva
Stack tecnologico
go
Ambito
api, backend

Direzione di ricerca

Start with base/base.go and the classification tests in base/action_test.go; trace how Action, MaxRetries and transport errors handle method names and request timing. Check the existing copy tests and public API retry behavior described in the issue. Done when the acceptance criteria pass for the listed methods, including OutcomeUnknown and errors.Is checks.

Scritto dal modello di indicizzazione a partire dal testo della issue.

Descrizione

bug

Context

This is a follow-up to Backblaze/blazer#167 (merged, closes #165). Since #167, base.Action returns Retry for 408 on every non-upload method and for 429, 500, 502, 503 and 504 on every method, including calls that are not safe to repeat: b2_create_key, b2_copy_file, b2_hide_file, b2_finish_large_file, b2_delete_file_version, b2_update_bucket and others. A 502 or 504 comes from a gateway and does not say whether B2 applied the request, so a write that already succeeded is sent again. base.Action, base.MaxRetries and base.Backoff decide this for the b2 package and for programs that use base directly, and they never look at whether the method is safe to repeat. This issue asks that the retries added in #167 (and the existing 500 and transport retries) be limited to calls that are safe to repeat.

Current behavior

At a5e0d13:

  • base/base.go:94-100: 408 returns Retry for every method except b2_upload_file/b2_upload_part (94-98); 429, 500, 502, 503 and 504 return Retry for every method (99-100). 5xx and transport failures on uploads return AttemptNewUpload (71-73, 76-78), which can store a second version.
  • base/base.go:518-558: every transport failure except certificate verification errors, including a reset or timeout after the request was written, becomes b2err{retry: 1} (552-556), so Action returns Retry (69-75); the method name is cleared for non-upload calls (532-534).
  • base/base.go:213-224: MaxRetries is 20 for uploads and downloads and 5 for everything else. base/base.go:230-242: Backoff is the retry value in seconds, so 1 s for transport errors.
  • Tests added in #167 expect this for b2_copy_part and b2_copy_file: base/action_test.go:69-99, b2/copy_test.go:117-126, b2/compose_extended_wire_test.go:740 and 742.

Reproduced at a5e0d13 through the public b2 API with an httptest server that fails the first call once and then succeeds:

status   b2_create_key  b2_copy_file  b2_hide_file  b2_update_bucket  b2_delete_file_version
408      2, err=nil     2, err=nil    2, err=nil    2, err=nil        2, err=nil
500      2, err=nil     2, err=nil    2, err=nil    2, err=nil        2, err=nil
502      2, err=nil     2, err=nil    2, err=nil    2, err=nil        2, err=nil
504      2, err=nil     2, err=nil    2, err=nil    2, err=nil        2, err=nil

Before #167 (7888c5b), the same test sent each call once for 408, 502 and 504 and returned the error. If the first b2_create_key was applied, the second creates a second key; the caller only ever sees one secret.

Expected behavior

A failure on a call that is not safe to repeat, after the request may have reached B2, is reported as "outcome unknown" and not re-sent automatically. Failures before the request was sent, 429 and 503 stay retryable, and #167's retries stay for calls that are safe to repeat.

Acceptance criteria

  • R1. A 500, 502, 504 or a mid-request connection close on b2_create_key, b2_hide_file, b2_copy_file, b2_finish_large_file and b2_delete_file_version gives OutcomeUnknown, MaxRetries == 0 and errors.Is(err, base.ErrOutcomeUnknown).
  • R2. A refused connection, a 429 and a 503 on the same methods give Retry.
  • R3. b2_list_file_names, b2_get_file_info and downloads keep Retry for 408, 500, 502, 503 and 504.

In scope

Proposed change
  • Keep the method name and the original error on every b2err.
  • Record whether the failure happened before send (dial, DNS, TLS handshake) or after (reset, EOF, read timeout).
  • Export an idempotency table, for example base.Idempotent(method string) bool: reads, upload URL calls, authorize, b2_set_bucket_notification_rules and downloads are idempotent; create key/bucket, hide, copy file, start/finish/cancel large file, deletes and b2_update_bucket with ifRevisionIs are not. If repeating b2_copy_part with the same part number simply replaces the part (not confirmed here), it belongs on the idempotent list and the Compose retries from #167 stay.
  • Add an ErrAction such as OutcomeUnknown and a sentinel base.ErrOutcomeUnknown for a post-send failure or a 500, 502 or 504 on a non-idempotent method; MaxRetries returns 0 for it. If B2 never returns 408 after applying a request (not confirmed here), 408 can stay Retry.
  • For uploads, keep AttemptNewUpload for pre-send failures and 503, document that other retries may create a second version, and let callers opt out (useful with Object Lock).

Out of scope

  • Unrelated work outside the components and behavior described in In scope.

Verification

Run the repository regression checks with live credentials unset:

unset B2_APPLICATION_KEY_ID B2_APPLICATION_KEY B2_ACCOUNT_ID B2_ACCOUNT_KEY
go build ./...
go vet ./...
go test -race -count=1 ./base ./b2/...
  • Use the supplied status-method matrix and controlled pre-send/post-send transport failures. Assert Action, MaxRetries, errors.Is, and exact request counts for the acceptance cases.
  • Preserve the stated read and 429/503 controls. Record the unresolved 408/copy-part safety decisions without treating their assumptions as already confirmed.
  • Expected result: the checks above succeed and the observable outcomes in Acceptance criteria are demonstrated. If a check cannot run, report Could not run because: ...; if it starts but fails, report Could not complete because: .... A skipped check is not a pass.

Links

Backblaze/blazer#167 (merged, closes #165) added the 408, 502 and 504 retries described above. Backblaze/blazer#101 (open: 401 unauthorized treated as an expired token) covers only the reauthorization replay. Backblaze/blazer#145 (merged) synchronized authorization state and did not change classification. Backblaze/blazer#18 and #31 bounded the retry count. Backblaze/blazer#117 and #129 added calls that go through the same retry path.

Lingua principale
Go
Stelle
66
Fork
18
Merge medio
1g 10h
PR unite (30g)
132

Preparare l'ambiente

Come iniziare

  1. Leggi tutta la issue e poi la guida ai contributi del progetto.
  2. Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
  3. Fai un fork del repository e lavora su un branch.
  4. Apri una pull request che faccia riferimento al numero della issue.

Altre issue di Backblaze/blazer

Tutte le issue di Backblaze/blazer

Issue simili

Altre issue su Go

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.