Skip to content

refactor(logs): one give-up budget for a --follow stream, not two - #611

Open
davidsu wants to merge 1 commit into
mainfrom
fix/logs-one-give-up-budget
Open

refactor(logs): one give-up budget for a --follow stream, not two#611
davidsu wants to merge 1 commit into
mainfrom
fix/logs-one-give-up-budget

Conversation

@davidsu

@davidsu davidsu commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Note

Description

base44 logs --follow tracked streaming failure in two independent counters: a connect-retry ladder (CONNECT_RETRY_DELAYS_MS, reset on every call) and a dead-stream budget (dropsSinceLastEvent, capped at 2 for the whole session). Since #595 made both give-up paths end the command identically, the split bought nothing and disagreed in both directions — the ladder could end a tail while the drop budget still held a spare life, and a backend alternating the two failure kinds could keep the CLI reconnecting forever because each counter was reset by the other's failure. This collapses them into a single strikes budget, where a strike is any attempt that produced no logs.

Related Issue

Follow-up to #595 (feat(logs): true realtime --follow via apper SSE stream). No linked issue.

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Refactoring (no functional changes)
  • Other (please describe): behaviour-affecting refactor — reconnect thresholds and pauses change, see "Additional Notes"

Changes Made

  • Replaced dropsSinceLastEvent with a single strikes counter shared by both failure kinds (failed connect, and stream that opened then delivered nothing).
  • RECONNECT_DELAYS_MS ([1s, 2s, 4s, 8s]) now doubles as both the backoff ladder and the give-up limit via hasRunOutOfStrikes().
  • connectWhileTransientlyUnavailableconnectWithinStrikes(filters, strikes): continues the caller's budget instead of starting a fresh one, and returns the strikes it spent so streamUntilExhausted carries them forward.
  • New Reconnected result type (stream | refused | exhausted) so an exhausted budget is distinguishable from a refusing backend.
  • strikesAfter(ending, strikes): a server-side rollover (event: end, retriable: true) wipes the budget to zero; a stream that proved itself alive (a row or a keepalive) costs exactly one strike rather than earning a clean slate, so a flapping backend can't spin indefinitely.
  • The post-stream reconnect pause is now the ladder rung for the current strike count instead of a flat STREAM_RECONNECT_DELAY_MS.
  • Deleted MAX_DROPS_SINCE_LAST_EVENT, STREAM_RECONNECT_DELAY_MS, countDropTowardGivingUp, shouldGiveUpStreaming, connectWhileTransientlyUnavailable, and the now-unused LogStreamAttempt import.
  • Rewrote tests/cli/logs.spec.ts's "stream drop budget" suite as "stream give-up budget" against the new helpers, adding cases for a budget shared across failure kinds and for rollover wiping it.

Testing

  • I have tested these changes locally
  • I have added/updated tests as needed
  • All tests pass (npm test)

Checklist

  • My code follows the project's style guidelines
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation (if applicable)
  • My changes generate no new warnings
  • I have updated docs/ (AGENTS.md) if I made architectural changes

Additional Notes

Observable behaviour changes, all inside the --follow reconnect loop:

  • The dead-stream threshold moves from 2 to the shared budget of 4 attempts, since drops no longer get their own small budget. This mainly helps quiet apps, where a stream can legitimately send nothing for its first ~20s (apper's KEEPALIVE_SECONDS) and a multi-pod rolling deploy could otherwise burn the cap of 2.
  • The initial connect is now at most 4 attempts over ~7s of backoff (was 5 attempts over ~15s) before falling back to polling.
  • The pause before reconnecting after a stream ends now scales with the strike count (still 1s when the budget is clean, as before).

Naming intentionally reads as prose (hasRunOutOfStrikes, ONE_STRIKE_FOR_A_STREAM_THAT_WORKED) to keep the retry policy legible at the call site. No file under docs/ describes this internal retry policy, so none needed updating.

⚠️ The streaming reconnect path still has no end-to-end coverage — pre-existing, and unchanged by this PR. Coverage here is unit-level on the pure helpers (strikesAfter / hasRunOutOfStrikes).


🤖 Generated by Claude | 2026-08-31 10:55 UTC | 90f0255

The follow path counted failure twice. The connect ladder counted
"couldn't open a stream" with a limit of five attempts and a lifetime of
one call, so it forgot everything between reconnects. dropsSinceLastEvent
counted "opened one and got nothing out of it" with a limit of two and a
lifetime of the whole session. Neither knew about the other, and after
the fail-fast change both ended the command the same way -- so the two
limits bought nothing and disagreed in both directions: the ladder could
end a tail while the drop budget still held a spare life, and a backend
that alternated the two failures could keep the CLI reconnecting forever
because each counter kept being reset by the other's kind of failure.

One counter now. A strike is any attempt that produced no logs, whether
it failed to connect or connected and delivered nothing. The ladder
doubles as the limit, connectWithinStrikes continues the caller's budget
instead of starting a fresh one, and a server-side rollover still wipes
it. A stream that proved itself alive costs one strike, never a clean
slate, so a flapping backend cannot spin here indefinitely.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

🚀 Package Preview Available!


Install this PR's preview build with npm:

npm i @base44-preview/cli@0.1.12-pr.611.23d31b2

Prefer not to change any import paths? Install using npm alias so your code still imports base44:

npm i "base44@npm:@base44-preview/cli@0.1.12-pr.611.23d31b2"

Or add it to your package.json dependencies:

{
  "dependencies": {
    "base44": "npm:@base44-preview/cli@0.1.12-pr.611.23d31b2"
  }
}

Preview published to npm registry — try new features instantly!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant