fix(core): contain thrown prewarm errors in the cachekeep tick - #155
Open
iceteaSA wants to merge 1 commit into
Open
fix(core): contain thrown prewarm errors in the cachekeep tick#155iceteaSA wants to merge 1 commit into
iceteaSA wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
No issues found across 2 files
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Architecture diagram
sequenceDiagram
participant Start as start()
participant Tick as tick()
participant Prewarm as prewarm()
participant Send as sendPrewarm()
participant Fetch as fetchImpl()
participant Cache as cacheExpiresAt
participant Publish as publishTrackedSessions()
Note over Start,Publish: CacheKeep Tick Failure-Containment Flow
Start->>Tick: schedule tick (60s)
Tick->>Tick: loadStorage()
loop each tracked target
Tick->>Tick: check target.cacheExpiresAt <= dueAt
alt target is due
Tick->>Prewarm: prewarm(target, now)
Prewarm->>Send: sendPrewarm(target)
Send->>Fetch: fetch(url, { signal: AbortSignal.timeout(...) })
alt fetch resolves successfully
Fetch-->>Send: Response
Send-->>Prewarm: { ok: true, ... }
Prewarm-->>Tick: ok result
Tick->>Cache: advance cacheExpiresAt
else fetch throws (timeout/network)
Fetch-->>Send: throws TimeoutError/TypeError
Send-->>Prewarm: { ok: false, transient: true }
Prewarm->>Cache: apply backoff (transient branch, not delete)
Prewarm-->>Tick: transient failure result
Note over Tick,Cache: Target retained, backed off, not deleted
end
else unexpected throw from prewarm
Tick->>Tick: catch error, log warning
Tick->>Cache: apply defensive backoff
end
end
Tick->>Publish: publishTrackedSessions()
Note over Send,Prewarm: Discriminator: status == null && !transient
alt unbuildable body (status == null, not transient)
Prewarm->>Prewarm: delete tracked target
end
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #150.
sendPrewarm()callsfetchwithsignal: AbortSignal.timeout(...), which throws on timeout or a network error instead of returning a result. Nothing between that throw andstart()'s.catch()handled it, so a single throwing target aborted the entiretick():And it never recovered: the backoff assignment lives in the
!result.okbranch that a throw skips, so the failing target'scacheExpiresAtwas never advanced and it stayed first-due, re-aborting every subsequent tick. Later targets were starved until they aged out as stale ~2h later, andpublishTrackedSessions()never ran, so the cross-process lease stopped refreshing.The change
sendPrewarm()catches the fetch throw and returns{ ok: false, reason, transient: true }.prewarm()routes transient failures to the existing backoff branch instead of the delete branch.tick()wraps eachprewarm()call defensively, applying the same backoff, so a future unanticipated throw cannot reintroduce head-of-line blocking.The delete path was the trap here.
prewarm()usedresult.status == nullto mean "unbuildable body, drop the tracked session", so mapping a thrown fetch onto that shape would have made a transient network blip permanently delete a live session — worse than the bug being fixed. Hence the separatetransientdiscriminator:transient: trueis set at exactly one site, and the delete predicate is nowresult.status == null && !result.transient. Unbuildable bodies still delete; HTTP-status failures still back off, unchanged.Two things worth flagging
prewarmNow()is a behavior change for consumers. It previously rejected on a fetch throw; it now resolves{ ok: false, transient: true }. The only in-repo consumer (warmFableAfterOpus) already handles both, but this is a public surface of@cortexkit/anthropic-auth-core, so it's a contract change worth knowing about.loadStorage()at the top oftick()is still unguarded and can abort a tick. I left it alone: it's pre-existing,start()'s.catch()handles it, it retries in 60s, and skipping a pass when the schedule can't be read is arguably right. Happy to wrap it if you'd rather.Verification
New tests assert that a throwing target does not prevent later targets from being prewarmed, that the throwing target is retained rather than deleted, that it backs off instead of retrying on the next tick, and that an unbuildable body still deletes its target.
Proven red before green — reverting the source change makes the new tests fail (2 fail, with the unbuildable-delete test correctly still passing as a regression guard). Gates on the branch:
bun run typecheckclean,bun run test1025 pass / 0 fail,bun run lintclean.Two other defects in this file are filed separately — #149 (failing prewarms retried at a fixed cadence forever) and #151 (ticks overlapping into duplicate paid prewarms). Kept independent so they can be triaged on their own; happy to send PRs for either if you want them.
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Summary by cubic
Contain thrown prewarm errors during cachekeep ticks to prevent head-of-line blocking and session starvation. Previously, a timeout/network error from
fetchinsendPrewarm()threw and aborted the entiretick(); now these errors are caught, treated as transient failures, and the tick proceeds to later targets.Behavior changes and migration
sendPrewarm()catchesfetchthrows and returns{ ok: false, reason, transient: true };prewarm()treats transient failures as backoff, not delete;tick()wraps per-target calls to apply the same backoff on unexpected throws.prewarmNow()no longer rejects on network/timeout errors. It resolves a failure result withtransient: true. Migration: update callers to checkresult.ok(and optionallyresult.transient) instead of catching a rejection.status == nulland not transient). Transient/network failures never delete tracked sessions.Written for commit e9afa64. Summary will update on new commits.
Greptile Summary
The PR contains fetch failures within cachekeep prewarming so one failed target no longer aborts the tick or starves later targets.
Confidence Score: 5/5
The PR appears safe to merge, with the changed failure paths preserving target lifecycle semantics while preventing one prewarm failure from aborting the entire tick.
The implementation distinguishes transient fetch failures from unbuildable requests, applies retry backoff without deleting live targets, continues processing later targets, and retains the existing deletion behavior for invalid prewarm bodies.
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD T[Cachekeep tick] --> D{Target is due?} D -- No --> N[Check next target] D -- Yes --> P[Build and send prewarm] P --> S{Result} S -- Success --> U[Advance cache expiry] S -- HTTP or transient failure --> B[Apply retry backoff] S -- Unbuildable body --> X[Delete tracked target] P -- Unexpected throw --> C[Log and apply defensive backoff] U --> N B --> N X --> N C --> N N --> F[Publish tracked sessions]Reviews (1): Last reviewed commit: "fix(core): contain thrown prewarm errors..." | Re-trigger Greptile
Context used: