fix(google-gemini): two HIGH-severity cache-checkpoint error-handling fixes (follow-up to #641)#643
Open
sroussey wants to merge 2 commits into
Open
Conversation
The Gemini cache-checkpoint warm-up wrapped ai.caches.create in a blanket try/catch that logged and swallowed every error, so a caller-initiated abort, a 401/403 auth failure, a 429 quota, and a 5xx transport error all silently degraded to the same "no cache, replay inline" fate as a genuine prefix-too- small 400. It also failed to clean up when a resource had been minted but a downstream step (bookkeeping / write) threw, leaving a server-side CachedContent billing until TTL. Classify the error into abort / degrade / throw. Abort and throw rethrow (with a best-effort delete of any resource whose name we saw); only a 400 / INVALID_ARGUMENT whose message names the too-small-prefix / cache-unsupported condition degrades to inline replay. Track `createdName` across the try / catch so the partial-delete cleanup can fire. Gemini_CacheStore gets a matching `createdAtMs` field on each entry, an `isGeminiCacheEntryStale` helper (default 3_500_000 ms — a hair under the 1h TTL used at write time), and a `deleteGeminiCachedContentLocal` for runtime-only eviction. These land here so PR #641's follow-up (H2, the consume-side fallback) can chain on top. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UC9LGu3iokhuhAB46yt1ee
…achedContent Gemini's explicit CachedContent is TTL-bound (~1h at creation) and can also vanish server-side outside the consumer's control. The text.generation and tool-calling run-fns treated the cache handle as guaranteed to resolve: an entry near-TTL still went out with `cachedContent`, and a NOT_FOUND surfaced as a hard failure to the caller. Add Gemini_CachedContentFallback: a proactive stale check (evicts the runtime-local entry before the request, leaves the server-side to its own TTL) plus a reactive NOT_FOUND catch that evicts (locally + best-effort server delete), rebuilds the request inline (prefix + tail), and retries once. Refactor both run-fns to share `buildCachedRequest` / `buildInlineReplayRequest` locals so the retry rebuilds an identical shape. Streaming contract unchanged (providers still yield text-delta/object-delta then finish; no accumulation). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UC9LGu3iokhuhAB46yt1ee
Coverage Report
File CoverageNo changed files found. |
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
2 participants
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.
Summary
Two HIGH-severity fixes to the Gemini cache-checkpoint error-handling paths introduced in #641.
H1 — Narrow the warm-up catch (
Gemini_CacheCheckpoint). The warm-up wrappedai.caches.createin a blanket try/catch that logged and swallowed every error, so a caller-initiated abort, a 401/403 auth failure, a 429 quota, and a 5xx transport error all silently degraded to the same "no cache, replay inline" fate as a genuine prefix-too-small 400. IntroduceclassifyGeminiCacheError(abort / degrade / throw): abort and throw rethrow with a best-effort delete of any resource whose name was returned before the failure; only a 400 / INVALID_ARGUMENT whose message names the too-small-prefix / unsupported condition degrades to inline replay.Gemini_CacheStoregets a matchingcreatedAtMson each entry, anisGeminiCacheEntryStalehelper (default 3_500_000 ms — a hair under the 1h write-time TTL), and adeleteGeminiCachedContentLocalfor runtime-only eviction, so H2 can chain on top.H2 — Fallback in the two consume paths (
Gemini_TextGeneration,Gemini_ToolCalling). The consumers treated the cache handle as guaranteed to resolve: a near-TTL entry still went out withcachedContent, and a NOT_FOUND surfaced as a hard failure to the caller. Extract the inline-replay branch into a named localbuildInlineReplayRequestin each run-fn and route both requests through a newGemini_CachedContentFallback.generateGeminiStreamWithCacheFallback. It runs a proactive stale check up front (evicts the runtime-local entry, leaves the server-side to its own TTL) and, on a reactive 404 /NOT_FOUND, evicts (locally + best-effort server delete), rebuilds inline, and retries once. The streaming contract is unchanged — providers still yieldtext-delta/object-deltaand a final emptyfinish; the consumer accumulates.Test plan
bun test packages/test/src/test/ai-provider/OpenAIGeminiCheckpointParams.test.ts— 24 tests green, adds 7 new cases (H1-abort, H1-degrade-preserved, H1-throws-429, H1-partial-delete; H2-NOT_FOUND-Text, H2-NOT_FOUND-ToolCalling, H2-proactive-stale) via the existingfakeGeminiClientseam.bun test packages/test/src/test/ai-provider/Gemini_SessionDispose.test.ts— no regressions.bun test packages/test/src/test/ai/CacheCheckpoint.test.ts— no regressions.bun run build:types— clean across all packages.Base
Stacks on #641 (like #642) — targets
ai-provider-cache-checkpoints, notmain. Merge #641 first; this branch rebases onto the resulting main state.🤖 Generated with Claude Code
https://claude.ai/code/session_01UC9LGu3iokhuhAB46yt1ee
Generated by Claude Code