fix(compression): pass a URL object when spawning the LLMLingua worker - #13093
Merged
diegosouzapw merged 1 commit intoSep 11, 2026
Merged
diegosouzapw merged 1 commit into
diegosouzapw merged 1 commit into
Conversation
ensureWorker() called `new Worker(pathToFileURL(file).href, ...)`, which passes
a STRING. node:worker_threads treats a string argument as a filesystem path and
requires it to start with ./ or ../, so a "file://..." string is looked up
literally and throws ERR_WORKER_PATH. Only a URL instance is interpreted as a
file: URL.
The failure was silent: pump() wraps ensureWorker() in `catch {}` and fails open,
so every compression call degraded to a passthrough while reporting success.
Verified against a real Worker on Node 24: the .href spelling throws
ERR_WORKER_PATH, the URL object spawns cleanly.
Fixes diegosouzapw#12822
Contributor
Author
|
Note on the checks here: the workflows are sitting in Locally on Windows 11 / Node 24.19.0: |
This was referenced Sep 9, 2026
diegosouzapw
added a commit
to insoln/OmniRoute
that referenced
this pull request
Sep 16, 2026
Reconcile with diegosouzapw#13093 (LLMLingua URL-object spawn fix) and diegosouzapw#13091 (idle-worker terminate() / remove(slot) signature): keep this PR's llmlinguaWorkerSpecifier()+workerFactory seam (superset of diegosouzapw#13093's inline fix — also testable and wired to notifyCompressionFailOpen), and drop the removed terminate boolean from every remove(slot) call site, including the close() drain path this PR adds. Update diegosouzapw#13093's own source-pattern regression test (llmlingua-worker-spawn-12822.test.ts) to match the new call shape — it still asserts the same contract (a URL instance, never .href, is passed to the worker), just via llmlinguaWorkerSpecifier() instead of inlined pathToFileURL(). Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
muhamadgalihsaputra
pushed a commit
to niyatna/NiyatnaRoute
that referenced
this pull request
Sep 27, 2026
diegosouzapw#13093) `node:worker_threads` only treats a `URL` instance as a `file:` URL — a string must be a relative path. The silent `catch {}` in `pump()` made this degrade compression to a passthrough while still reporting success, which is the worst shape for it. --- Validated in one consolidated worktree cut from `release/v3.8.51`, boarded together with the other 13 PRs of this batch — zero merge conflicts between them. - `typecheck:core` clean - complexity 2799 / baseline 3218 and cognitive-complexity 1265 / baseline 1437 — both under baseline - 71 focused assertions green across the 13 test files this batch adds or touches⚠️ base-red inherited: diegosouzapw#12732 — `Docs Gates (fast-path)`, `Merge integrity`, `No new ESLint warnings`, `Unit Tests fast-path` and `Fast Quality Gates` all reproduce on the pure `release/v3.8.51` tip (provider count 356 vs the 358 the modules define, SKILL.md drift, and `open-sse/utils/stream.ts` at 3115 > frozen 3098). None of them touch this diff. Thanks @anhtahaylove — the root-cause write-up, the measured before/after numbers and the red-before-green proof on every one of these made the batch reviewable as a unit.
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.
ensureWorker()callednew Worker(pathToFileURL(absoluteWorkerFile).href, { execArgv }), which passes a string.node:worker_threadstreats a string argument as a filesystem path and requires it to start with./or../; only aURLinstance is interpreted as afile:URL. So the"file://..."text was looked up literally as a relative path and the spawn threwERR_WORKER_PATH.The failure was silent.
pump()wrapsensureWorker()in a barecatch {}and fails open, so every compression call quietly degraded to a passthrough while still reporting success — the worst shape for this bug, since nothing in the logs or the response says compression did not happen.Confirmed against a real
Workeron Node 24, isolated from this codebase:The change
One argument: drop
.hrefand pass theURLobject.pathToFileURLstays — the sibling testllmlingua-worker-resolution.test.tsasserts this file never touchesimport.meta.urlorcreateRequire(both die in the standalone webpack bundle), andpathToFileURLis a pure path→URL converter with neither dependency. Passing the raw path instead would also work on Node, but the URL form is what keeps spaces and non-ASCII in the install path unambiguous.Test
tests/unit/compression/llmlingua-worker-spawn-12822.test.tsasserts the source does not spawn with.href, and separately proves the runtime contract by spawning both spellings against a real worker file — so it stays honest if the call is ever reshaped rather than just matching text.Verification
Fixes #12822