fix: use native fetch for Ollama embedding to ensure AbortController works#383
Conversation
…works Root cause: OpenAI SDK HTTP client does not reliably abort Ollama TCP connections when AbortController.abort() fires in Node.js. This causes stalled sockets that hang until the gateway-level 120s timeout. Fix: Add isOllamaProvider() to detect localhost:11434 endpoints, and embedWithNativeFetch() using Node.js 18+ native fetch instead of the OpenAI SDK. Native fetch properly closes TCP connections on abort. Also adds Test 8 to cjk-recursion-regression.test.mjs with assertion updated to match Embedding provider unreachable error (AliceLJY review fix). Fixes CortexReach#361.
Summary of ChangesThis is a clean rebase of PR #362 addressing your review feedback: Applied fixes from your CI diagnosis:
Retained:
Excluded:
Original PR #362 has been closed. CI should now pass. Thanks for the detailed diagnosis @AliceLJY! |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6782a1dfee
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // Use an unreachable port + localhost so isOllamaProvider() returns true | ||
| // (URL contains 127.0.0.1:11434) but nothing actually listens there. | ||
| // This forces native fetch to properly reject, validating the Ollama path. | ||
| const ollamaBaseURL = "http://127.0.0.1:11434/v1"; |
There was a problem hiding this comment.
This test is meant to verify that abort signals propagate through the Ollama native-fetch path, but it hardcodes http://127.0.0.1:11434/v1 instead of using the baseURL from withServer. As written, the request usually fails immediately with connection refusal, so the slow handler (and its abort/destroy behavior) is never exercised; the test can pass even if abort propagation is broken.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
分析完全正確，已修復 ✅
作者的診斷非常精準：
http://127.0.0.1:11434/v1hardcoded，請求永遠立即 connection refused，根本沒打到 slow handler。
問題根因：withServer() 分配隨機 port，但 Embedder 仍指向 127.0.0.1:11434，導致請求從未接觸到 mock server 的 11 秒延遲邏輯。
新 commit (a26fd70) 修復內容：
-
src/embedder.ts：withTimeout()接受外部AbortSignal，合併至內部 AbortController。embedQuery/embedPassage全部支援signal?: AbortSignal並傳到底層。 -
test/cjk-recursion-regression.test.mjs：testOllamaAbortWithNativeFetch()現在：- Mock server 直接綁定
127.0.0.1:11434（讓isOllamaProvider()= true） - Server 延遲 5 秒回應
embedPassage(text, externalSignal)在 2 秒時 abort- 斷言：耗時 ≈ 2s（不是 5s） — 證明 abort 打斷了慢請求
- Mock server 直接綁定
-
test/embedder-ollama-abort.test.mjs：獨立測試檔，隔離性更好。
測試結果
✔ Ollama embedWithNativeFetch aborts slow request within expected time (2029.86ms)
PASSED (aborted in 2029ms < 5000ms threshold)
Note on CI failure: reflection-bypass-hook.test.mjsThe cli-smoke failure in this PR comes from test/reflection-bypass-hook.test.mjs, which is not part of our changes (this PR only touches src/embedder.ts and est/cjk-recursion-regression.test.mjs). The test failures in reflection-bypass-hook are pre-existing on master. The error:
appears to be caused by a date-based seed: the test uses \Date.UTC(2026, 2, 12)\ for seed data, but the current date is 2026-03-26+. This causes derived memory entries to exceed the age filter threshold and get filtered out, returning empty results. Suggested fix (for a separate PR)Update the hardcoded seed date in \ est/reflection-bypass-hook.test.mjs\ to a date within the active window (e.g., \Date.UTC(2026, 2, 26)\ or use a relative date). I'll open a separate issue + PR for this. The Ollama fix itself (embedder.ts changes) is ready and unrelated to this failure. |
PR Chain UpdateSee #362 for the full chain of related issues and PRs:
|
|
新增的 如果你想保留当前这个测试形态，我建议把测试名/注释收窄成“验证 Ollama URL 会走 native fetch 路径”；如果想把它作为这个修复的核心回 |
|
分析完全正確。已在新 commit 中修復： 問題根因 修復內容
驗證結果 |
2 participants
Problem
When using Ollama as the embedding provider, the \embedQuery\ timeout does not reliably abort stalled Ollama HTTP requests, causing the gateway-level 120s timeout.
Root Cause
OpenAI SDK HTTP client does not reliably abort Ollama TCP connections when AbortController.abort() fires in Node.js.
Fix
Use native \ etch\ for Ollama endpoints. Node.js 18+ native fetch correctly respects AbortController.
Test
Fixes #361.