mirror of
https://github.com/QwenLM/qwen-code.git
synced 2026-10-19 12:43:39 +00:00
fix(core): quiet the crawler's ripgrep probe and stop double-reporting one failed spawn (#14573)
When `rg` is not installed, the file-search crawler printed an alarming
block of raw failure diagnostics for a single failed spawn, and printed
it twice:
[crawler] command spawn failed: rg --files --no-require-git --hidden ENOENT Error: spawn rg ENOENT
[crawler] falling back to fdir (ripgrep unavailable)
[crawler] command failed: rg --files --no-require-git --hidden exit=-2
Two defects, fixed separately:
- The ripgrep probe is an optional-capability probe whose failure is
already narrated by the "falling back to fdir (ripgrep unavailable)"
warn, so its raw spawn diagnostics are pure noise. Pass
`silentOnFailure: true`, matching the three git probes that already do.
- `runCommand` consulted its `settled` flag only inside `finalize()`,
which guards `resolve()`. The logging sites sit outside that guard, so
when a spawn fails the `error` handler logs "command spawn failed" and
the `close` handler then re-reports the very same failure as "command
failed ... exit=-2" (libuv surfacing UV_ENOENT as the close code on
Linux). Add a `!settled` leg to the close handler's condition.
Deliberately out of scope: routing the probe through a resolved ripgrep
binary, and memoizing "rg is unavailable". The `git ls-files --cached`
call is also left non-silent on purpose - its degradation is already
narrated, and keeping it loud leaves the double-report observable.
Refs #14537
Patrol-Run: qwen-issue-patrol/jmvdlnotoa2
Co-authored-by: yiliang114 <yiliang114@users.noreply.github.com>
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
This commit is contained in:
parent
67717f1c89
commit
ab52c896ed
2 changed files with 155 additions and 1 deletions
|
|
@ -951,6 +951,153 @@ describe('crawler', () => {
|
|||
]);
|
||||
warnSpy.mockRestore();
|
||||
});
|
||||
|
||||
/**
|
||||
* Makes the *real* `runCommand` hit a genuine `spawn` ENOENT partway
|
||||
* through a crawl, without stubbing `__setCommandRunnerForTests`.
|
||||
*
|
||||
* `crawlWithGitLsFiles` reads `options.ignore.getDirectoryFilter()` after
|
||||
* `rev-parse` and the `ls-files --others` / `--deleted` probes have already
|
||||
* spawned successfully, but before it spawns `ls-files --cached` and before
|
||||
* the ripgrep probe. Clearing `PATH` at that point makes those two spawns
|
||||
* really fail with ENOENT — reproducing #14537's "`rg` is not installed"
|
||||
* condition on any machine, including CI runners that do ship `rg`.
|
||||
*
|
||||
* Returns a restore function; callers must run it in a `finally`.
|
||||
*/
|
||||
const breakSpawnMidCrawl = (ignore: Ignore): (() => void) => {
|
||||
const savedPath = process.env['PATH'];
|
||||
const realDirectoryFilter = ignore.getDirectoryFilter.bind(ignore);
|
||||
let pathDropped = false;
|
||||
const filterSpy = vi
|
||||
.spyOn(ignore, 'getDirectoryFilter')
|
||||
.mockImplementation(() => {
|
||||
if (!pathDropped) {
|
||||
pathDropped = true;
|
||||
process.env['PATH'] = '';
|
||||
}
|
||||
return realDirectoryFilter();
|
||||
});
|
||||
return () => {
|
||||
process.env['PATH'] = savedPath;
|
||||
filterSpy.mockRestore();
|
||||
};
|
||||
};
|
||||
|
||||
/**
|
||||
* `[crawler] command …` diagnostics matching `needle`, in order.
|
||||
*
|
||||
* Matched against *all* console.warn output on purpose: the `degradations()`
|
||||
* helper filters to `[crawler] falling back to …`, which discards exactly the
|
||||
* diagnostic lines under test here.
|
||||
*/
|
||||
const commandDiagnostics = (warns: string[], needle: RegExp) =>
|
||||
warns.filter((w) => w.startsWith('[crawler] command') && needle.test(w));
|
||||
|
||||
/**
|
||||
* Crawls a real git repo whose `git`/`rg` spawns genuinely fail mid-crawl,
|
||||
* returning every `console.warn` line the crawl produced.
|
||||
*/
|
||||
const crawlWithGenuineSpawnFailure = async (ignore: Ignore) => {
|
||||
const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {});
|
||||
const restoreSpawn = breakSpawnMidCrawl(ignore);
|
||||
try {
|
||||
const results = await crawlTmp(ignore);
|
||||
return {
|
||||
results,
|
||||
warns: warnSpy.mock.calls.map((c) => String(c[0])),
|
||||
};
|
||||
} finally {
|
||||
restoreSpawn();
|
||||
warnSpy.mockRestore();
|
||||
}
|
||||
};
|
||||
|
||||
it('should not emit raw spawn diagnostics when the ripgrep probe fails (#14537)', async () => {
|
||||
tmpDir = await createTmpDir({ 'tracked.js': '' });
|
||||
await initGitRepo(tmpDir);
|
||||
const ignore = rules(tmpDir);
|
||||
|
||||
const { results, warns } = await crawlWithGenuineSpawnFailure(ignore);
|
||||
|
||||
// Silencing the probe loses no information: the crawl still succeeds via
|
||||
// fdir and the degradation itself is still narrated.
|
||||
expect(results).toContain('tracked.js');
|
||||
expect(warns).toContain(
|
||||
'[crawler] falling back to fdir (ripgrep unavailable)',
|
||||
);
|
||||
expect(commandDiagnostics(warns, /: rg(\s|$)/)).toEqual([]);
|
||||
});
|
||||
|
||||
it('should report a single failed spawn once, not twice (#14537)', async () => {
|
||||
tmpDir = await createTmpDir({ 'tracked.js': '' });
|
||||
await initGitRepo(tmpDir);
|
||||
const ignore = rules(tmpDir);
|
||||
|
||||
const { results, warns } = await crawlWithGenuineSpawnFailure(ignore);
|
||||
|
||||
expect(results).toContain('tracked.js');
|
||||
expect(warns).toContain(
|
||||
'[crawler] falling back to ripgrep (git ls-files unavailable)',
|
||||
);
|
||||
// `ls-files --cached` is deliberately *not* silent, so this one failed
|
||||
// spawn stays observable: it must be reported exactly once. Before the
|
||||
// fix the `error` handler logs `command spawn failed` and the `close`
|
||||
// handler re-reports the same failure as `command failed … exit=-2`
|
||||
// (libuv surfacing UV_ENOENT as the close code).
|
||||
const diagnostics = commandDiagnostics(warns, /ls-files/);
|
||||
expect(diagnostics).toHaveLength(1);
|
||||
expect(diagnostics[0]).toContain('command spawn failed');
|
||||
});
|
||||
|
||||
it('should suppress the close-handler diagnostic for a silent probe that exits non-zero (#14537)', async () => {
|
||||
tmpDir = await createTmpDir({ 'plain.js': '' });
|
||||
// Precondition guard, so the assertion below cannot pass vacuously:
|
||||
// tmpDir really is outside any git repo, so `findGitRoot` really spawns
|
||||
// git and really gets a non-zero *exit* — a different failure shape from
|
||||
// the spawn ENOENT above, and the one where `silentOnFailure` (not
|
||||
// `settled`) is the deciding leg of the close-handler guard.
|
||||
await expect(
|
||||
runExecFile('git', ['rev-parse', '--show-toplevel'], tmpDir),
|
||||
).rejects.toThrow();
|
||||
|
||||
const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {});
|
||||
let results: string[] = [];
|
||||
let warns: string[] = [];
|
||||
try {
|
||||
results = await crawlTmp(rules(tmpDir));
|
||||
warns = warnSpy.mock.calls.map((c) => String(c[0]));
|
||||
} finally {
|
||||
warnSpy.mockRestore();
|
||||
}
|
||||
|
||||
expect(results).toContain('plain.js');
|
||||
expect(commandDiagnostics(warns, /: git(\s|$)/)).toEqual([]);
|
||||
});
|
||||
|
||||
it('should stay quiet when the non-silent git listing succeeds (#14537)', async () => {
|
||||
tmpDir = await createTmpDir({ 'tracked.js': '' });
|
||||
await initGitRepo(tmpDir);
|
||||
|
||||
const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {});
|
||||
let results: string[] = [];
|
||||
let warns: string[] = [];
|
||||
try {
|
||||
results = await crawlTmp(rules(tmpDir));
|
||||
warns = warnSpy.mock.calls.map((c) => String(c[0]));
|
||||
} finally {
|
||||
warnSpy.mockRestore();
|
||||
}
|
||||
|
||||
expect(results).toContain('tracked.js');
|
||||
// Precondition guard: nothing fell back, so the git path really succeeded
|
||||
// and the *non-silent* `ls-files --cached` really exited 0 — the case
|
||||
// where `!ok` is the deciding leg of the close-handler guard.
|
||||
expect(
|
||||
warns.filter((w) => w.startsWith('[crawler] falling back to')),
|
||||
).toEqual([]);
|
||||
expect(commandDiagnostics(warns, /.*/)).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
||||
describe('throttling', () => {
|
||||
|
|
|
|||
|
|
@ -594,7 +594,10 @@ function runCommand(
|
|||
flushRemainder();
|
||||
|
||||
const ok = code === 0;
|
||||
if (!ok && !options?.silentOnFailure) {
|
||||
// `!settled` skips a failure the `error` handler already reported: when a
|
||||
// spawn fails, libuv surfaces UV_ENOENT as the close code (-2 on Linux),
|
||||
// so without this the same single failure is logged twice (#14537).
|
||||
if (!ok && !options?.silentOnFailure && !settled) {
|
||||
logCommandProblem('command failed', command, args, {
|
||||
code,
|
||||
stderr: stderrBuf,
|
||||
|
|
@ -1351,6 +1354,10 @@ async function crawlWithRipgrep(
|
|||
const rgResult = await commandRunner('rg', rgArgs, crawlDirectory, 20_000, {
|
||||
collectLines: false,
|
||||
onLine: processRgFile,
|
||||
// An optional-capability probe: its failure is already narrated by the
|
||||
// "falling back to fdir (ripgrep unavailable)" warn below, so the raw
|
||||
// spawn diagnostics would only be noise (#14537).
|
||||
silentOnFailure: true,
|
||||
});
|
||||
|
||||
if (!rgResult.success) {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue