Skip to content

fix: bind valid slices after earlier errors - #3183

Open
lowgame wants to merge 1 commit into
labstack:masterfrom
lowgame:fix-binder-valid-slices-8fd0e18
Open

lowgame wants to merge 1 commit into
labstack:masterfrom
lowgame:fix-binder-valid-slices-8fd0e18

Conversation

@lowgame

@lowgame lowgame commented Oct 18, 2026

Copy link
Copy Markdown

Summary

When FailFast(false) collected an error for one field, later valid numeric, boolean, float, time, and duration slice fields were parsed but left unchanged. This change lets each slice binding assign its result when that specific conversion adds no errors, while retaining atomic no-assignment when an element in the current slice is invalid.

Reproduction

A query containing bad=nope&ints=1&ints=2 followed by:

QueryParamsBinder(c).FailFast(false).
    Int("bad", &bad).
    Ints("ints", &ints).
    BindErrors()

returned the expected error for bad, but ints remained at its previous value. The regression test reproduced the same behavior across integer, unsigned integer, boolean, float, time, and duration slices before the production change.

Root cause

The slice converters assigned their temporary result only when the binder's complete error list was nil. Under FailFast(false), that list can contain an unrelated earlier error even when the current slice converted successfully.

Changes

  • Compare the error count before and after each slice conversion.
  • Assign the converted slice only when the current conversion added no errors.
  • Add regression coverage for all affected slice converter families.

No public API or dependency changes are introduced; documentation is unchanged because this restores the documented FailFast(false) behavior.

Validation

  • go test ./... -run '^TestValueBinder_FailFastFalseBindsValidSlicesAfterError$' -count=1 — pass
  • go test ./... -count=1 — pass
  • make vet — pass
  • go test -race ./... — pass
  • make lint — pass
  • git diff --check — pass

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant