mirror of
https://github.com/heygen-com/hyperframes.git
synced 2026-09-08 02:36:10 +00:00
fix(hyperframes-media): surface a clear error when npx can't be resolved on Windows (#1961)
On Windows, resolveSpawnCommand routes `npx` through node + npm's
npx-cli.js (avoiding the un-spawnable npx.cmd), locating that CLI via
npm_execpath. When the script is run directly with `node audio.mjs`
instead of through npm/npx, npm_execpath is unset, so resolution returns
null and spawnP short-circuited to `{status:-1}` — silently. With
stdio:"ignore" hiding everything, callers just reported "TTS failed -
omitted" for every single line, giving no hint that the real cause was
an unresolvable npx. Debugging required reading the source.
Fix: when spawnP hits that null-resolution path, emit a clear one-time
diagnostic naming npm_execpath and the remedy (run via npx/npm, or export
npm_execpath) before returning {status:-1}. One-shot latch so a batch of
lines logs it once, not per line. Behavior is otherwise unchanged — still
returns {status:-1} and spawns nothing.
Test: new tts.spawn.test.mjs case — two consecutive win32 npx calls with
npm_execpath unset both return {status:-1}, nothing is spawned, and the
diagnostic (mentioning npm_execpath) is emitted exactly once. Existing
spawn tests unchanged (7/7 pass).
This commit is contained in:
@@ -155,6 +155,13 @@ export function resolveSpawnCommand(
|
||||
// `platform`/`spawnFn` params (default process.platform / the real spawn)
|
||||
// exist so tests can exercise the win32 branch without mocking node:child_process
|
||||
// (its ESM exports are non-configurable, so mock.method can't patch it).
|
||||
// One-shot so a whole batch of TTS lines doesn't repeat the same diagnostic.
|
||||
let _warnedNpxResolution = false;
|
||||
/** Test-only: reset the one-shot npx-resolution warning latch. */
|
||||
export function _resetNpxResolutionWarnForTests() {
|
||||
_warnedNpxResolution = false;
|
||||
}
|
||||
|
||||
export function spawnP(
|
||||
cmd,
|
||||
args,
|
||||
@@ -165,7 +172,23 @@ export function spawnP(
|
||||
pathExists = existsSync,
|
||||
) {
|
||||
const resolved = resolveSpawnCommand(cmd, args, opts, platform, env, pathExists);
|
||||
if (!resolved) return Promise.resolve({ status: -1 });
|
||||
if (!resolved) {
|
||||
// resolveSpawnCommand only returns null for the npx-on-win32 case where
|
||||
// npm_execpath isn't set (e.g. audio.mjs invoked directly with `node`, not
|
||||
// through npm/npx). Without this, every call silently returns status:-1 and
|
||||
// stdio:"ignore" hides why — callers just report "TTS failed - omitted" for
|
||||
// every line. Surface the real reason once so it's diagnosable.
|
||||
if (!_warnedNpxResolution) {
|
||||
_warnedNpxResolution = true;
|
||||
console.error(
|
||||
`[hyperframes-media] Cannot run "${cmd}" on Windows: npm_execpath is not set, so the ` +
|
||||
`npx JS CLI can't be located. This happens when this script is run directly with ` +
|
||||
`\`node\` instead of through npm/npx. Every "${cmd}" call is being skipped. ` +
|
||||
`Fix: run via \`npx\`/\`npm run\`, or export npm_execpath pointing at your npm-cli.js.`,
|
||||
);
|
||||
}
|
||||
return Promise.resolve({ status: -1 });
|
||||
}
|
||||
return new Promise((resolve) => {
|
||||
const p = spawnFn(resolved.cmd, resolved.args, resolved.opts);
|
||||
p.on("exit", (code) => resolve({ status: code ?? -1 }));
|
||||
|
||||
@@ -1,7 +1,12 @@
|
||||
import { test } from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
import { EventEmitter } from "node:events";
|
||||
import { resolveNpxCliFromNpmExecPath, resolveSpawnCommand, spawnP } from "./tts.mjs";
|
||||
import {
|
||||
resolveNpxCliFromNpmExecPath,
|
||||
resolveSpawnCommand,
|
||||
spawnP,
|
||||
_resetNpxResolutionWarnForTests,
|
||||
} from "./tts.mjs";
|
||||
|
||||
// Regression: on Windows, npx resolves to npx.cmd, which spawn() cannot exec
|
||||
// without shell:true — it fails ENOENT, silently swallowed as ok:false by the
|
||||
@@ -95,3 +100,44 @@ test("spawnP does not enable shell for non-npx commands even on win32", async ()
|
||||
assert.deepEqual(captured[0].args, ["-c", "pass"]);
|
||||
assert.equal(captured[0].opts.shell, undefined);
|
||||
});
|
||||
|
||||
// Regression: win32 + npx with npm_execpath unset can't locate the npx JS CLI,
|
||||
// so resolveSpawnCommand returns null and spawnP short-circuits. Previously it
|
||||
// returned {status:-1} silently — every TTS line just dropped as "TTS failed -
|
||||
// omitted" with no hint. Now it must surface a clear one-time diagnostic naming
|
||||
// npm_execpath, while still returning {status:-1} without spawning anything.
|
||||
test("spawnP surfaces a clear diagnostic (once) when npx can't be resolved on win32", async () => {
|
||||
_resetNpxResolutionWarnForTests();
|
||||
const errors = [];
|
||||
const originalError = console.error;
|
||||
console.error = (msg) => errors.push(msg);
|
||||
const captured = [];
|
||||
const emptyEnv = {}; // no npm_execpath
|
||||
try {
|
||||
const r1 = await spawnP(
|
||||
"npx",
|
||||
["hyperframes", "tts"],
|
||||
{},
|
||||
"win32",
|
||||
fakeSpawn(captured),
|
||||
emptyEnv,
|
||||
() => false,
|
||||
);
|
||||
const r2 = await spawnP(
|
||||
"npx",
|
||||
["hyperframes", "tts"],
|
||||
{},
|
||||
"win32",
|
||||
fakeSpawn(captured),
|
||||
emptyEnv,
|
||||
() => false,
|
||||
);
|
||||
assert.equal(r1.status, -1);
|
||||
assert.equal(r2.status, -1);
|
||||
assert.equal(captured.length, 0, "must not spawn anything when resolution fails");
|
||||
assert.equal(errors.length, 1, "diagnostic is emitted once for a batch, not per line");
|
||||
assert.match(errors[0], /npm_execpath/);
|
||||
} finally {
|
||||
console.error = originalError;
|
||||
}
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user