hyperledger-firefly / hyperledger-firefly/common

Flaky CI in `pkg/eventstreams`: webhook retry loop races test teardown (connection refused on localhost)

Open
#236 0 comments 0 reactions 0 assignees View on GitHub
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.