fix: reset the streaming reconnect budget after a healthy session

[karlwaldman]

Closes #34

The defect

MaxReconnectAttempts is documented as "the number of consecutive reconnect attempts before the stream terminates". run() incremented one counter at stream.go:351 and never reset it, so it was really a lifetime disconnect count. A stream that recovered fully, ran healthily, and dropped again later spent budget it had already earned back — and eventually terminated a subscription that was working.

The comment on connectAndRead even claimed the opposite ("A successful subscription resets the caller's backoff counter"). Nothing implemented it.

Verified still reproducing on main (cd51e3a), matching the evidence in the issue:

stream_reconnect_budget_test.go:140: confirmed subscriptions = 2, want the 3 staged healthy sessions;
the reconnect budget was not restored by a healthy session
(terminating error: stream reconnect failed after 1 attempt(s): websocket: close 1006 (abnormal closure): unexpected EOF)

The healthy-progress boundary

The issue's own constraint: "Define a meaningful healthy-progress boundary rather than resetting on TCP dial and creating an infinite rapid-flap loop."

A session is healthy once the subscription has been confirmed AND at least one further server frame — an ActionCable ping or a channel message — has arrived on that same connection.

Boundary considered	Why not
TCP dial succeeded	A server that accepts and hangs up resets every pass. Unbounded flap.
Handshake / welcome received	Same: welcome arrives before the client even subscribes.
confirm_subscription alone	A server that confirms and instantly drops resets every pass. Still an unbounded flap — and this is a real failure mode when a subscription is accepted but the backend behind it is unhealthy.
Confirmed + one further server frame	Proves a live session rather than a reachable socket. The server sends it unprompted (ActionCable pings every few seconds), so it needs no cooperation from the caller, and it is deterministic to test — no sleeps.

A time-based threshold was rejected as the primary rule: it makes the test timing-dependent and it picks an arbitrary number that is wrong for any server whose legitimate session length sits near it.

The change

• connectAndRead returns (healthy bool, err error); run() resets both the attempt count and the backoff when healthy is true.
• handleFrame returns the frame kind alongside the fatal error, so the accounting reads frames already being parsed rather than parsing them twice.
• A server-initiated disconnect notice is explicitly not healthy progress — the server is telling us to go away.
• MaxReconnectAttempts and WithStreamMaxReconnectAttempts now document the reset semantics; the unlimited-retry opt-in (a negative value) is untouched.

Red

Tests written first, run against unmodified main:

=== RUN   TestStreamHealthySessionResetsReconnectBudget
    stream_reconnect_budget_test.go:140: confirmed subscriptions = 2, want the 3 staged healthy sessions; the reconnect budget was not restored by a healthy session (terminating error: stream reconnect failed after 1 attempt(s): websocket: close 1006 (abnormal closure): unexpected EOF)
--- FAIL: TestStreamHealthySessionResetsReconnectBudget (0.00s)
=== RUN   TestStreamConfirmThenImmediateDropDoesNotResetBudget
--- PASS: TestStreamConfirmThenImmediateDropDoesNotResetBudget (0.00s)
=== RUN   TestStreamRejectedSubscriptionStillTerminatesWithoutRetrying
--- PASS: TestStreamRejectedSubscriptionStillTerminatesWithoutRetrying (0.00s)
=== RUN   TestStreamCloseDuringBackoffLeavesNoGoroutine
--- PASS: TestStreamCloseDuringBackoffLeavesNoGoroutine (0.51s)
FAIL
FAIL	github.com/OilpriceAPI/oilpriceapi-go	0.526s

The three passing tests are the negative paths the issue asks for. They are green before and after by design — they exist to prove the fix does not buy the reset by breaking the cap, the fatal-rejection rule, or teardown.

The tests drive a real httptest WebSocket server whose per-connection behaviour is scripted by connection index, so what is asserted is the number of subscriptions the server actually saw and the stream's terminating error — not an internal counter.

Green

=== RUN   TestStreamHealthySessionResetsReconnectBudget
--- PASS: TestStreamHealthySessionResetsReconnectBudget (0.01s)
=== RUN   TestStreamConfirmThenImmediateDropDoesNotResetBudget
--- PASS: TestStreamConfirmThenImmediateDropDoesNotResetBudget (0.00s)
=== RUN   TestStreamRejectedSubscriptionStillTerminatesWithoutRetrying
--- PASS: TestStreamRejectedSubscriptionStillTerminatesWithoutRetrying (0.00s)
=== RUN   TestStreamCloseDuringBackoffLeavesNoGoroutine
--- PASS: TestStreamCloseDuringBackoffLeavesNoGoroutine (0.51s)
PASS
ok  	github.com/OilpriceAPI/oilpriceapi-go	0.534s

Acceptance criteria from the issue

Criterion	Covered by
Fail first with a healthy session, disconnect, another healthy session, then a failure	TestStreamHealthySessionResetsReconnectBudget (red above)
Consecutive failed attempts stop at the cap; qualifying recovery resets budget/backoff	same test (terminates with reconnect failed once the server stops recovering) + TestStreamConfirmThenImmediateDropDoesNotResetBudget
Immediate flapping	TestStreamConfirmThenImmediateDropDoesNotResetBudget — at most 1 initial + 2 attempts
Permanent auth/subscription rejection	TestStreamRejectedSubscriptionStillTerminatesWithoutRetrying — exactly 1 subscribe, no retry
Context cancellation / Close during backoff; no goroutine leak	TestStreamCloseDuringBackoffLeavesNoGoroutine, plus the existing TestStreamContextCancelTeardown and TestStreamCloseIsIdempotent
Document reset semantics, preserve unlimited-retry opt-in	doc comments on StreamOptions.MaxReconnectAttempts, WithStreamMaxReconnectAttempts, connectAndRead; negative value untouched
go test ./... and race tests	below

Full suite

Check	Result
go test ./...	ok github.com/OilpriceAPI/oilpriceapi-go 17.679s — 289 pass / 0 fail (285 baseline + 4 new)
go test -race ./...	ok github.com/OilpriceAPI/oilpriceapi-go 19.226s
go test -race -count=5 -run TestStream .	ok github.com/OilpriceAPI/oilpriceapi-go 3.786s (flake check on the timing-sensitive stream tests)
go vet ./...	clean
gofmt -l .	clean

Scope

handleFrame and connectAndRead are unexported; no public API changes. No version bump, no tag, no release.

🤖 Generated with Claude Code

https://claude.ai/code/session_015ao5paex73xXvuM424Libo

[coderabbitai]

Important

• 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 5a6f3300-c286-454d-affb-f48ec44e308c

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}