mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-08-19 05:32:19 +03:00
fix(quality): green release/v3.8.50 unit base-reds (#9985)
8 unit-test base-reds reproducing on the pristine release tip, fixed in-source (fast-gates PR->release do not run the unit suite, so these accrued silently): - ServiceSupervisor: spawn-failure now resolves with error status (was throwing); health-probe-failure path still rejects. Distinct via spawnFailed flag. - stream + responseSanitizer: numeric passthrough id preserved as string (was regenerated chatcmpl-); finish chunk with empty delta no longer swallowed by the emptyChoices guard. - proxyFetch: genuine (non-abort) proxy transport failures keep the underlying reason in the surfaced error. - auto-combo builtinCatalog: advertised undefined-variant auto/* ids (auto/chat, auto/best-chat, auto/pro-chat) materialize instead of throwing 'Unknown'. - getTranslations en.json: add missing providers.iconUrlInvalid. - optional-transformers-dependency.test: reconcile to #9962's deliberate move of @huggingface/transformers to a regular dep (napi onnxruntime). Co-authored-by: OmniRoute maintenance <maintainers@omniroute.local>
This commit is contained in:
@@ -576,15 +576,21 @@ function sanitizeResponsesUsage(usage: unknown): unknown {
|
||||
|
||||
/**
|
||||
* Normalize response ID to use chatcmpl- prefix.
|
||||
* Preserves numeric/short custom ids as their string form rather than
|
||||
* regenerating them — a passthrough numeric id (e.g. `123`) must stay `"123"`
|
||||
* so streaming clients can correlate chunks (#3427/#5776). Only a genuinely
|
||||
* missing/empty id gets a fresh `chatcmpl-` token.
|
||||
*/
|
||||
function normalizeResponseId(id: unknown): string {
|
||||
if (!id || typeof id !== "string") {
|
||||
if (!id || (typeof id !== "string" && typeof id !== "number")) {
|
||||
return `chatcmpl-${crypto.randomUUID().replace(/-/g, "").slice(0, 29)}`;
|
||||
}
|
||||
// Already correct format
|
||||
if (id.startsWith("chatcmpl-")) return id;
|
||||
// Keep custom IDs but don't break them
|
||||
return id;
|
||||
const str = String(id);
|
||||
if (str === "") {
|
||||
return `chatcmpl-${crypto.randomUUID().replace(/-/g, "").slice(0, 29)}`;
|
||||
}
|
||||
// Already correct format, or a custom/numeric id — keep it.
|
||||
return str;
|
||||
}
|
||||
|
||||
function normalizeResponsesId(id: unknown): string {
|
||||
|
||||
@@ -182,6 +182,16 @@ export async function createBuiltinAutoCombo(modelStr: string, suffix: string) {
|
||||
return virtualCombo;
|
||||
}
|
||||
|
||||
// Advertised `auto/*` ids whose template maps to no variant (auto/chat,
|
||||
// auto/best-chat, auto/pro-chat) still materialize via the default
|
||||
// (unconstrained) virtual combo rather than throwing "Unknown built-in".
|
||||
if (Object.prototype.hasOwnProperty.call(AUTO_TEMPLATE_VARIANTS, modelStr)) {
|
||||
const virtualCombo = await createVirtualAutoCombo(undefined);
|
||||
virtualCombo.name = modelStr;
|
||||
virtualCombo.id = modelStr;
|
||||
return virtualCombo;
|
||||
}
|
||||
|
||||
// #4235 Phase B: `auto/<category>[:<tier>]` (e.g. auto/coding:fast, auto/vision).
|
||||
const parsed = parseAutoSuffix(suffix);
|
||||
if (parsed.valid) {
|
||||
|
||||
@@ -1096,9 +1096,12 @@ async function patchedFetch(
|
||||
continue;
|
||||
}
|
||||
tagProxyUnreachable(error);
|
||||
const originalMsg = error instanceof Error ? error.message : String(error);
|
||||
const sanitized = sanitizeTransportError(
|
||||
error,
|
||||
"Proxy request failed",
|
||||
originalMsg
|
||||
? `Proxy request failed: ${originalMsg}`
|
||||
: "Proxy request failed",
|
||||
"PROXY_REQUEST_FAILED"
|
||||
);
|
||||
console.error(
|
||||
|
||||
@@ -1640,7 +1640,8 @@ export function createSSEStream(options: StreamOptions = {}) {
|
||||
(parsed.choices.length === 1 &&
|
||||
parsed.choices[0]?.delta &&
|
||||
typeof parsed.choices[0].delta === "object" &&
|
||||
Object.keys(parsed.choices[0].delta).length === 0))
|
||||
Object.keys(parsed.choices[0].delta).length === 0 &&
|
||||
!parsed.choices[0]?.finish_reason))
|
||||
) {
|
||||
const emptyChoicesUsage = extractUsage(parsed) ?? parsed.usage;
|
||||
if (hasValidUsage(emptyChoicesUsage)) {
|
||||
@@ -1850,6 +1851,7 @@ export function createSSEStream(options: StreamOptions = {}) {
|
||||
passthroughSawFinishReason = true;
|
||||
}
|
||||
|
||||
|
||||
if (isFinishChunk && passthroughHasToolCalls) {
|
||||
toolFinishTime = now;
|
||||
try {
|
||||
|
||||
@@ -4948,6 +4948,7 @@
|
||||
"baseUrlHint": "Required. Provider API base URL.",
|
||||
"iconUrlLabel": "Icon URL",
|
||||
"iconUrlHint": "Optional. Image URL shown as this provider's icon.",
|
||||
"iconUrlInvalid": "Invalid icon URL. Use an http(s):// or data:image/*;base64 URL.",
|
||||
"anthropicPrefixPlaceholder": "ac-prod",
|
||||
"openaiPrefixPlaceholder": "oc-prod",
|
||||
"anthropicBaseUrlPlaceholder": "https://api.anthropic.com/v1",
|
||||
|
||||
@@ -46,6 +46,7 @@ export class ServiceSupervisor extends EventEmitter {
|
||||
private lastError: string | null = null;
|
||||
private childProcess: ChildProcess | null = null;
|
||||
private adopted: boolean = false;
|
||||
private spawnFailed: boolean = false;
|
||||
private readonly buffer: RingBuffer;
|
||||
private readonly checker: HealthChecker;
|
||||
private operationLock: Promise<void> = Promise.resolve();
|
||||
@@ -158,6 +159,7 @@ export class ServiceSupervisor extends EventEmitter {
|
||||
child = spawn(command, args, buildServiceSpawnOptions(env, cwd));
|
||||
} catch (err) {
|
||||
this.checker.stop();
|
||||
this.spawnFailed = true;
|
||||
const msg = sanitizeErrorMessage(err instanceof Error ? err.message : String(err));
|
||||
this.lastError = msg;
|
||||
this.setState("error");
|
||||
@@ -195,6 +197,7 @@ export class ServiceSupervisor extends EventEmitter {
|
||||
// the health poller hammers the dead port every healthIntervalMs.
|
||||
child.once("error", (err) => {
|
||||
this.checker.stop();
|
||||
this.spawnFailed = true;
|
||||
const msg = sanitizeErrorMessage(err instanceof Error ? err.message : String(err));
|
||||
this.lastError = msg;
|
||||
this.setState("error");
|
||||
@@ -206,6 +209,12 @@ export class ServiceSupervisor extends EventEmitter {
|
||||
|
||||
await this.waitForHealthy();
|
||||
|
||||
// A spawn failure flips state to "error" — surface the explicit error
|
||||
// status instead of overriding it with "running".
|
||||
if (this.state === "error") {
|
||||
return this.getStatus();
|
||||
}
|
||||
|
||||
this.setState("running");
|
||||
await setToolStatus(this.config.tool, "running", this.pid ?? undefined);
|
||||
|
||||
@@ -260,13 +269,21 @@ export class ServiceSupervisor extends EventEmitter {
|
||||
|
||||
while (Date.now() < deadline) {
|
||||
if (this.checker.getHealth() === "healthy") return;
|
||||
if (this.state === "error") throw new Error(this.lastError ?? "Service failed to start");
|
||||
// A spawn failure (child 'error' event or sync throw) flips state to
|
||||
// "error" — stop polling and let start() surface the explicit error
|
||||
// status as a resolve. A health-probe failure is a different, harder
|
||||
// condition and must reject (handled below).
|
||||
if (this.state === "error") {
|
||||
if (this.spawnFailed) return;
|
||||
throw new Error(this.lastError ?? "Service failed to start");
|
||||
}
|
||||
await new Promise((r) => setTimeout(r, 1_000));
|
||||
}
|
||||
// Timeout reached without a healthy probe. The health poller may have
|
||||
// flipped the state to "error" while we were waiting (FAILURE_THRESHOLD
|
||||
// consecutive failures) — surface that instead of a degraded marker.
|
||||
if (this.state === "error") {
|
||||
if (this.spawnFailed) return;
|
||||
throw new Error(this.lastError ?? "Service failed to start");
|
||||
}
|
||||
this.lastError = sanitizeErrorMessage(
|
||||
|
||||
@@ -9,7 +9,14 @@ function readJson<T = Record<string, unknown>>(relPath: string): T {
|
||||
return JSON.parse(readFileSync(join(repoRoot, relPath), "utf8")) as T;
|
||||
}
|
||||
|
||||
test("@huggingface/transformers is optional so onnxruntime CUDA install failures cannot abort OmniRoute install", () => {
|
||||
test("@huggingface/transformers is a regular dependency so npm ci never skips it", () => {
|
||||
// #9962 deliberately moved @huggingface/transformers out of optionalDependencies:
|
||||
// as an optional dep, npm silently skipped the whole subtree on Node 24/26 (old
|
||||
// pin dragged onnxruntime-node@1.21.0 whose NAN build no longer compiles), which
|
||||
// broke `npm ci`/`next build` with "Can't resolve @huggingface/transformers"
|
||||
// (lazy import in src/lib/memory/embedding/transformersLocal.ts). As a regular
|
||||
// dep with onnxruntime-node@~1.24.3 (napi prebuilds, no node-gyp) it stays
|
||||
// installable and the memory embedding path requires() cleanly.
|
||||
const pkg = readJson<{
|
||||
dependencies?: Record<string, string>;
|
||||
optionalDependencies?: Record<string, string>;
|
||||
@@ -17,29 +24,37 @@ test("@huggingface/transformers is optional so onnxruntime CUDA install failures
|
||||
|
||||
assert.equal(
|
||||
pkg.dependencies?.["@huggingface/transformers"],
|
||||
undefined,
|
||||
"transformers must not be a regular dependency because it pulls onnxruntime-node install scripts"
|
||||
"^4.2.0",
|
||||
"transformers must be a regular dependency (never optional) so npm ci cannot skip it"
|
||||
);
|
||||
assert.equal(pkg.optionalDependencies?.["@huggingface/transformers"], "3.5.2");
|
||||
assert.equal(pkg.optionalDependencies?.["@huggingface/transformers"], undefined);
|
||||
});
|
||||
|
||||
test("package-lock marks transformers and its onnxruntime runtime as optional", () => {
|
||||
test("transformers + onnxruntime-node are regular dependencies (not optional)", () => {
|
||||
const pkg = readJson<{
|
||||
dependencies?: Record<string, string>;
|
||||
optionalDependencies?: Record<string, string>;
|
||||
}>("package.json");
|
||||
|
||||
assert.equal(
|
||||
pkg.dependencies?.["onnxruntime-node"],
|
||||
"~1.24.3",
|
||||
"onnxruntime-node is a regular dep (napi prebuilds, installable on Node 24/26)"
|
||||
);
|
||||
assert.equal(pkg.optionalDependencies?.["onnxruntime-node"], undefined);
|
||||
|
||||
const lock = readJson<{
|
||||
packages: Record<string, { optional?: boolean; dependencies?: Record<string, string>; optionalDependencies?: Record<string, string> }>;
|
||||
}>("package-lock.json");
|
||||
|
||||
assert.equal(
|
||||
lock.packages[""]?.dependencies?.["@huggingface/transformers"],
|
||||
undefined,
|
||||
"root lock dependencies must not keep transformers as mandatory"
|
||||
"^4.2.0",
|
||||
"root lock dependencies must hold transformers as a regular (non-optional) dep"
|
||||
);
|
||||
assert.equal(lock.packages[""]?.optionalDependencies?.["@huggingface/transformers"], "3.5.2");
|
||||
|
||||
for (const packagePath of [
|
||||
"node_modules/@huggingface/transformers",
|
||||
"node_modules/onnxruntime-node",
|
||||
"node_modules/onnxruntime-common",
|
||||
]) {
|
||||
assert.equal(lock.packages[packagePath]?.optional, true, `${packagePath} should be optional`);
|
||||
}
|
||||
// Optional flag is only written `true` for genuinely optional packages;
|
||||
// regular deps leave it absent/null. Assert each is NOT optional.
|
||||
assert.ok(!lock.packages["node_modules/@huggingface/transformers"]?.optional, "transformers must not be marked optional in the lockfile");
|
||||
assert.ok(!lock.packages["node_modules/onnxruntime-node"]?.optional, "onnxruntime-node must not be marked optional in the lockfile");
|
||||
assert.ok(!lock.packages["node_modules/onnxruntime-common"]?.optional, "onnxruntime-common must not be marked optional in the lockfile");
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user