hyperledger-firefly / hyperledger-firefly/common
Flaky CI in `pkg/eventstreams`: webhook retry loop races test teardown (connection refused on localhost)
- Dominant language
- Go
- Stars
- 5
- Forks
- 15
- Avg merge
- 3d 6h
- Merged PRs (30d)
- 4
Description
## Summary
The Go CI job is intermittently failing in `github.com/hyperledger-firefly/common/pkg/eventstreams` due to a teardown race between webhook retry goroutines and test HTTP server shutdown.
- Failing job: https://github.com/hyperledger-firefly/common/actions/runs/29846418098/job/88688053787?pr=234
- PR: #234
- Package: `pkg/eventstreams`
- Ref seen in logs: `3abb2d84777e743eb570f1e560aab56b7317ea3b`
## Symptoms from logs
Repeated retry attempts continue dispatching PUT requests to a localhost mock endpoint after it is no longer accepting connections:
- `Put "http://127.0.0.1:45445/some/path": dial tcp 127.0.0.1:45445: connect: connection refused`
- `Batch attempt N failed. err=FF00219: Webhook request failed ...`
- Stream lifecycle churn appears concurrently (`stopped`, `stopping`, `deleted`, `No in-memory state for stream ...`)
The overall run fails with:
- `FAIL github.com/hyperledger-firefly/common/pkg/eventstreams`
- `make: *** [Makefile:15: test] Error 1`
## Expected behavior
Event stream tests should deterministically stop all dispatch/retry workers before teardown of test infrastructure (e.g., mock HTTP servers), so CI is stable.
## Proposed fix
1. Ensure stream stop is synchronous for tests (or provide a blocking test helper):
- cancel stream context
- wait for all worker goroutines to exit (`WaitGroup`)
2. In tests, stop stream before server shutdown:
- `require.NoError(t, es.Stop())` before `srv.Close()` / test exit
3. Use deterministic retry config in tests:
- low max attempts (1–2)
- short backoff
4. Assert on terminal error/state rather than timing-sensitive retry log counts.
## Suggested implementation direction
- If `Stop()` is currently non-blocking, make it block until all dispatch/retry loops terminate.
- Ensure every worker loop exits on `ctx.Done()` and decrements `WaitGroup`.
- Update flaky tests in `pkg/eventstreams` to enforce stop-before-close ordering.
## Notes
This appears to be test flakiness/race behavior, not a product logic regression.
Contributor guide
Research direction
Start in pkg/eventstreams by running its tests with the race detector and reviewing the failing CI run from PR #234. Trace Stop(), the webhook retry workers, and the flaky tests to verify cancellation and worker completion before mock server shutdown. Done means the package tests pass deterministically with workers stopped before teardown and without timing-sensitive retry assertions.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- testing
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 55/100