hyperledger / hyperledger/fabric-x
bug(fxconfig/provider): sync.Once permanently caches initialization failures with no retry capability
- Dominant language
- Go
- Stars
- 64
- Forks
- 80
- Avg merge
- 1d 22h
- Merged PRs (30d)
- 15
Description
In [`provider.go`](https://github.com/hyperledger/fabric-x/blob/main/tools/fxconfig/internal/provider/provider.go), the generic `Provider[T, K]` struct uses `sync.Once` (line 19) to lazily initialize service instances (orderer client, query client, notification client).
The `Get()` method (lines 43–51) runs the factory inside `once.Do()`:
```go
func (p *Provider[T, K]) Get() (T, error) {
p.once.Do(func() {
if err := p.cfg.Validate(p.validationContext); err != nil {
p.err = err
return
}
p.instance, p.err = p.factory(p.cfg)
})
return p.instance, p.err
}
```
Because `sync.Once` guarantees the function runs **exactly once**, if the first `Get()` call fails due to a **transient** error (e.g., temporary DNS failure, network partition, orderer not yet ready), the error is permanently cached in `p.err` and **every subsequent `Get()` call returns the same stale error forever**. There is no way to retry or reset the provider.
This makes any application using `fxconfig` unable to recover from transient startup failures without a full process restart.
## Steps to Reproduce
1. Create a `Provider` whose factory depends on an external service (e.g., orderer).
2. Ensure the external service is temporarily unavailable during the first `Get()` call.
3. Observe that `Get()` returns an error (expected).
4. Bring the external service back up.
5. Call `Get()` again.
## Actual Behavior
`Get()` immediately returns the previously cached error **without** re-invoking the factory. The application cannot recover.
## Expected Behavior
`Get()` should retry the factory on subsequent calls when the previous failure was due to a transient error, while still permanently caching failures from config validation errors (which won't resolve on retry).
## Proposed Fix
Replace `sync.Once` with a `sync.Mutex`-guarded lazy initialization pattern.
- On **transient** factory errors → allow retry on next `Get()` call.
- On **permanent** errors (e.g., `cfg.Validate()` failure) → cache and never retry.
Add unit tests verifying:
- Successful init is cached (no duplicate factory calls).
- Transient factory errors allow retry.
- Permanent validation errors are cached.
- Thread-safety under concurrent access.
Contributor guide
No contributing guide indexed for this repository
Assessment
This issue has not been assessed yet.