hyperledger / hyperledger/fabric-x

bug(fxconfig/provider): sync.Once permanently caches initialization failures with no retry capability

Open
#204 1 comment 0 reactions 0 assignees View on GitHub
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.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.