Repository navigation
fix(embedding): retry transient remote embedding failures (fixes import timeout #400) - #401
Merged
Merged
Conversation
Problem: a single hung remote embedding request aborted an entire import with zero memories written (issue tickernelz#400, reproduced 2x) and could wedge search the same way. The warmup probe had no retry either, and its failure set the sticky initError until restart. Design: extract the remote call into fetchRemoteEmbeddingWithRetry — per-attempt 10s ceiling, up to 3 attempts, linear backoff, retry only network errors/timeouts/429/5xx, fail fast on other 4xx. Both the warmup probe and embed() now route through it, so add, search-query, and import all gain transient-failure resilience inside the existing outer embedWithTimeout budget. Verification: new tests/embedding-remote-retry.test.ts (6 cases: hang-recovery, 429, 500, fail-fast 401, exhaustion, request shape); tsc --noEmit, eslint, prettier clean; full bun test failure set identical to clean main (15 pre-existing environmental failures). End-to-end: instrumented import with one injected 40s fetch hang — before: Timeout after 30000ms, imported 0; after: success, imported 3. Consequences: worst-case latency for a dead endpoint rises from 30s per call to ~31.5s across attempts but stays bounded; healthy-endpoint behavior unchanged (single attempt).
Use AbortSignal.timeout per attempt so hung fetches are cancelled instead of orphaned, keep 3×8s under the outer 30s budget, and whitelist only transient errors for retry (429/5xx, abort/timeout, network TypeError).
2 of 9 tasks
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.
Fixes #400.
Root cause
With remote embeddings configured, a single hung HTTP request anywhere in the import path aborted the whole import with
Timeout after 30000msand zero memories written. Two exposed spots:initializeModel()reachability probe: one fetch under the 30sTIMEOUT_MS, no retry — and its failure sets the stickyinitErroruntil restart.embed()remote fetch: no per-request timeout/retry; one hang inside import Phase A's sequential embed loop discarded all progress.Reproduced deterministically by injecting one 40s fetch hang:
{"success":false,"imported":0,"error":"Timeout after 30000ms"}.Fix
New
fetchRemoteEmbeddingWithRetry(): per-attempt 10s ceiling, up to 3 attempts, linear backoff, retry limited to network errors / per-attempt timeouts / 429 / 5xx; other 4xx fail fast. Both the probe andembed()route through it, so add, search-query, and import all gain transient-failure resilience while staying inside the existing outerembedWithTimeoutbudget.Evidence
{"success":true,"imported":3}in ~12s (hang absorbed on attempt 2).tests/embedding-remote-retry.test.ts(6 cases: hang recovery, 429, 500, fail-fast 401, attempt exhaustion, request shape).tsc --noEmit,eslint . --max-warnings=0, prettier clean; fullbun testfailure set identical to clean main in this environment (15 pre-existing environmental failures, none related).Notes