Skip to content

fix(bench): report validator init errors on every NewView call - #2580

Merged
Effi-S merged 1 commit into
mainfrom
fix-2565-validator-cache
Nov 1, 2026
Merged

Effi-S merged 1 commit into
mainfrom
fix-2565-validator-cache

Conversation

@AkramBitar

Copy link
Copy Markdown
Contributor

The benchmark built its validator inside once.Do but stored the error in a variable local to each NewView call, so only the first call could see it. If that first init failed, every later call skipped the error guard and got a nil validator — the real cause was lost and requests failed with a misleading "validator backend is nil".

validatorCache.get now returns the error on every call, and does not memoize failures. The cache is a field of the factory instead of a package global; its zero value works, so no call site changes.

Fixes #2565

@AkramBitar AkramBitar added this to the Q4/26 milestone Oct 29, 2026
@AkramBitar AkramBitar added bug Something isn't working benchmark hardening labels Oct 29, 2026
@AkramBitar AkramBitar self-assigned this Oct 29, 2026
@AkramBitar
AkramBitar requested review from Effi-S and adecaro and removed request for Effi-S October 29, 2026 12:29

@adecaro adecaro left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Effi-S Effi-S left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

The token validation benchmark built its validator inside `once.Do` but
recorded the error into an `initErr` local to each `NewView` call, so only
the first call could observe it. If that first init failed, `cachedValidator`
stayed nil and every later call skipped the `initErr` guard and handed the
view a nil validator. The real cause was lost and all subsequent requests
failed inside `Call` with a misleading "validator backend is nil".

Replace the `once`/`cachedValidator` globals with a `validatorCache` whose
`get` returns `(*token.Validator, error)` on every call. A failure is not
memoized, so a transient error does not poison the cache. The cache is a
field of the factory rather than a global; its zero value works, so every
existing call site is unchanged.

Also switch this file to the FSC `errors` package and document the validator
lifecycle.

Fixes #2565

Signed-off-by: AkramBitar <akram@il.ibm.com>
@Effi-S
Effi-S force-pushed the fix-2565-validator-cache branch from 7a98830 to 23a3fba Compare November 1, 2026 10:46
@Effi-S
Effi-S merged commit ae9df8d into main Nov 1, 2026
221 checks passed
@Effi-S
Effi-S deleted the fix-2565-validator-cache branch November 1, 2026 11:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

benchmark bug Something isn't working hardening

Projects

None yet

Development

Successfully merging this pull request may close these issues.

sync.Once init-error dropped, nil validator reused

3 participants