mirror of
https://github.com/morluto/rea.git
synced 2026-10-26 17:35:11 +00:00
fix(signals): report rejected sigrok selectors as invalid input (#5570)
sigrok-cli refusals of a caller-supplied decoder ID or annotation class were reported as execution_failure with "Run `rea doctor`" advice, which has no sigrok check. Recognize the observed sigrok-cli 0.7.2 refusals at the provider boundary only when the rejected name matches a caller selector, and report invalid_request naming the decoders or annotations argument, retaining the captured output and pointing to inspect_signal_decoders. Other non-zero exits stay provider failures. Fixes #5515 Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
c1e04bf8c0
commit
4dfe3464b2
3 files changed
+141
-2
No files matched your search
@@ -7,6 +7,10 @@ import {
|
||||
DecoderDiscoveryBudgetError,
|
||||
parseDecoderDiscovery,
|
||||
} from "./parseDecoderDiscovery.js";
|
||||
import {
|
||||
parseSigrokSelectorRejection,
|
||||
type SigrokSelectorRejection,
|
||||
} from "./sigrokSelectorRejection.js";
|
||||
import {
|
||||
BOUNDED_PRODUCER_LIMITS as LIMITS,
|
||||
prlimitArgumentVector,
|
||||
@@ -166,7 +170,12 @@ export class SigrokRecordingProvider implements SignalRecordingPort {
|
||||
root = await this.createRuntime();
|
||||
this.#resources.retain(root, undefined);
|
||||
const snapshotPath = join(root.path, "recording");
|
||||
const execute = async (args: readonly string[]) => {
|
||||
const execute = async (
|
||||
args: readonly string[],
|
||||
rejectedSelector?: (
|
||||
stderr: string,
|
||||
) => SigrokSelectorRejection | undefined,
|
||||
) => {
|
||||
capturedOutput = undefined;
|
||||
const output = await runOwnedCommand(
|
||||
{
|
||||
@@ -222,6 +231,16 @@ export class SigrokRecordingProvider implements SignalRecordingPort {
|
||||
);
|
||||
phase = previousPhase;
|
||||
resourcePath = "path" in input ? input.path : command;
|
||||
const rejection =
|
||||
output.exitCode === 0
|
||||
? undefined
|
||||
: rejectedSelector?.(output.stderr.text);
|
||||
if (rejection !== undefined)
|
||||
throw new AnalysisInputError(
|
||||
operation,
|
||||
{ capturedOutput, userMessage: rejection.message },
|
||||
[rejection],
|
||||
);
|
||||
if (output.exitCode !== 0)
|
||||
throw new ProviderAdapterError(
|
||||
SIGROK_PROVIDER_IDENTITY.id,
|
||||
@@ -311,7 +330,9 @@ export class SigrokRecordingProvider implements SignalRecordingPort {
|
||||
? []
|
||||
: [`--protocol-decoder-annotations=${input.annotations}`]),
|
||||
];
|
||||
const output = await execute(args);
|
||||
const output = await execute(args, (stderr) =>
|
||||
parseSigrokSelectorRejection(stderr, input),
|
||||
);
|
||||
phase = "integrity";
|
||||
resourcePath = snapshotPath;
|
||||
if (
|
||||
|
||||
@@ -0,0 +1,52 @@
|
||||
import type { AnalysisInputIssue } from "../domain/analysisErrorCore.js";
|
||||
import type { DecodeSignalRecordingInput } from "../domain/signals/signalRecording.js";
|
||||
|
||||
export type SigrokSelectorRejection = AnalysisInputIssue & {
|
||||
readonly message: string;
|
||||
};
|
||||
|
||||
const rejectedSelector = (
|
||||
field: "decoders" | "annotations",
|
||||
producerMessage: string,
|
||||
): SigrokSelectorRejection => ({
|
||||
path: [field],
|
||||
reason: "invalid_value",
|
||||
message: `sigrok-cli rejected the ${field} selector: ${producerMessage} Use inspect_signal_decoders to list installed decoder IDs, or pass a decoder ID to show its options and annotation classes.`,
|
||||
});
|
||||
|
||||
/**
|
||||
* Recognize only observed sigrok-cli 0.7.2 selector refusals whose rejected
|
||||
* name equals a caller-supplied selector; other failures stay unclassified.
|
||||
*/
|
||||
export const parseSigrokSelectorRejection = (
|
||||
stderr: string,
|
||||
input: Pick<DecodeSignalRecordingInput, "decoders" | "annotations">,
|
||||
): SigrokSelectorRejection | undefined => {
|
||||
const lines = new Set(stderr.split("\n"));
|
||||
// sigrok-cli stacks decoders with "," and appends options with ":".
|
||||
for (const entry of input.decoders.split(",")) {
|
||||
const id = entry.split(":")[0] ?? "";
|
||||
const producerMessage = `Failed to load protocol decoder ${id}.`;
|
||||
// A missing module named exactly after the decoder separates an unknown ID
|
||||
// from an installed decoder whose own imports or code fail to load.
|
||||
if (
|
||||
id !== "" &&
|
||||
lines.has(producerMessage) &&
|
||||
lines.has(
|
||||
`srd: ModuleNotFoundError: Failed to load decoder ${id}: import by name failed: No module named '${id}'`,
|
||||
)
|
||||
)
|
||||
return rejectedSelector("decoders", producerMessage);
|
||||
}
|
||||
// Annotation selections are "decoder=class:class,decoder".
|
||||
for (const entry of input.annotations?.split(",") ?? []) {
|
||||
const [decoder = "", classes, ...rest] = entry.split("=");
|
||||
if (decoder === "" || classes === undefined || rest.length > 0) continue;
|
||||
for (const annotation of classes.split(":")) {
|
||||
const producerMessage = `Annotation '${annotation}' not found for protocol decoder '${decoder}'.`;
|
||||
if (annotation !== "" && lines.has(producerMessage))
|
||||
return rejectedSelector("annotations", producerMessage);
|
||||
}
|
||||
}
|
||||
return undefined;
|
||||
};
|
||||
@@ -1,5 +1,6 @@
|
||||
import { AnalysisResourceConstraintError } from "../../../src/domain/analysisErrorCore.js";
|
||||
import { signalDecodersSchema } from "../../../src/domain/signals/signalDecoders.js";
|
||||
import { projectAnalysisError } from "../../../src/domain/analysisErrorProjection.js";
|
||||
import { OwnedCommandFailure } from "../../../src/process/OwnedCommand.js";
|
||||
import { it, expect, onTestFinished } from "vitest";
|
||||
import { writeFile, readdir, chmod, symlink } from "node:fs/promises";
|
||||
@@ -28,6 +29,9 @@ const fixture = async (mode = "ok", failCleanup = false) => {
|
||||
if (process.argv.includes('--version')) { console.log(${JSON.stringify(mode)} === 'bad-version' ? 'sigrok-cli 0.7.1' : 'sigrok-cli 0.7.2\\nlibsigrokdecode 0.5.3'); console.error('version warning'); }
|
||||
else if (${JSON.stringify(mode)} === 'bad-version') console.log('unexpected version fallback');
|
||||
else if (${JSON.stringify(mode)} === 'nonzero') { console.error('invalid decoder'); process.exitCode = 1; }
|
||||
else if (${JSON.stringify(mode)} === 'unknown-decoder') { console.error("srd: ModuleNotFoundError: Failed to load decoder nosuchdecoder: import by name failed: No module named 'nosuchdecoder'\\nFailed to load protocol decoder nosuchdecoder."); process.exitCode = 1; }
|
||||
else if (${JSON.stringify(mode)} === 'broken-decoder') { console.error("srd: ModuleNotFoundError: Failed to load decoder uart: import by name failed: No module named 'serial'\\nFailed to load protocol decoder uart."); process.exitCode = 1; }
|
||||
else if (${JSON.stringify(mode)} === 'unknown-annotation') { console.error("Annotation 'nosuchclass' not found for protocol decoder 'uart'."); process.exitCode = 1; }
|
||||
else if (${JSON.stringify(mode)} === 'malformed-copy') { chmodSync('recording', 0o600); writeFileSync('recording', 'changed'); }
|
||||
else if (${JSON.stringify(mode)} === 'oversized-copy') { chmodSync('recording', 0o600); truncateSync('recording', 32 * 1024 * 1024 + 1); }
|
||||
else if (${JSON.stringify(mode)} === 'line-budget') process.stdout.write('ID: uart' + '\\n'.repeat(65536));
|
||||
@@ -250,6 +254,68 @@ it.each([
|
||||
},
|
||||
);
|
||||
|
||||
it.each([
|
||||
[
|
||||
"unknown-decoder",
|
||||
{ decoders: "uart:rx=RX,nosuchdecoder" },
|
||||
"decoders",
|
||||
"Failed to load protocol decoder nosuchdecoder.",
|
||||
],
|
||||
[
|
||||
"unknown-annotation",
|
||||
{ decoders: "uart", annotations: "uart=rx-data:nosuchclass" },
|
||||
"annotations",
|
||||
"Annotation 'nosuchclass' not found for protocol decoder 'uart'.",
|
||||
],
|
||||
] as const)(
|
||||
"reports sigrok's %s refusal as invalid caller input",
|
||||
async (mode, selection, field, refusal) => {
|
||||
const f = await fixture(mode);
|
||||
const result = await f.service.decode({ path: f.path, ...selection });
|
||||
if (result.ok) throw new Error("Expected selector refusal");
|
||||
expect(result.error._tag).toBe("AnalysisInputError");
|
||||
expect(projectAnalysisError(result.error)).toMatchObject({
|
||||
code: "invalid_request",
|
||||
message: expect.stringContaining("inspect_signal_decoders"),
|
||||
details: {
|
||||
issues: [
|
||||
{
|
||||
path: [field],
|
||||
reason: "invalid_value",
|
||||
message: expect.stringContaining(refusal),
|
||||
},
|
||||
],
|
||||
captured_output: {
|
||||
exit_code: 1,
|
||||
stderr: expect.stringContaining(refusal),
|
||||
},
|
||||
},
|
||||
});
|
||||
expect(result.error.cleanupIncomplete).toBe(false);
|
||||
expect(
|
||||
(await readdir(f.directory)).filter((name) =>
|
||||
name.startsWith("decoder-"),
|
||||
),
|
||||
).toEqual([]);
|
||||
},
|
||||
);
|
||||
|
||||
it.each([
|
||||
// An installed decoder whose own dependency is missing is an installation fault.
|
||||
["broken-decoder", { decoders: "uart" }],
|
||||
// Refusals naming selectors the caller did not supply stay unclassified.
|
||||
["unknown-decoder", { decoders: "uart" }],
|
||||
["unknown-annotation", { decoders: "uart", annotations: "uart=rx-data" }],
|
||||
] as const)(
|
||||
"keeps %s outside caller selectors as a provider failure",
|
||||
async (mode, selection) => {
|
||||
const f = await fixture(mode);
|
||||
const result = await f.service.decode({ path: f.path, ...selection });
|
||||
if (result.ok) throw new Error("Expected decoder failure");
|
||||
expect(result.error._tag).toBe("ProviderAdapterError");
|
||||
},
|
||||
);
|
||||
|
||||
it.each(["decode", "discovery"] as const)(
|
||||
"does not attribute the version probe's output to a %s command that fails to launch",
|
||||
async (operation) => {
|
||||
|
||||
Reference in new issue
Block a user