mirror of
https://github.com/morluto/rea
synced 2026-10-25 22:08:11 -04:00
fix(electron): record actions skipped after a failed action (#6023)
An Electron scenario stopped at the first failed or cancelled action and returned no record for the remaining declared actions, so normalized_result.actions could not account for the whole scenario. Record every remaining action as cancelled with a not-executed error, the representation browser scenarios already use. Both runners now share one helper for that error, which also distinguishes a stop caused by request cancellation from one caused by an action failure. Fixes #5943 Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -212,7 +212,9 @@ statements remain under the attached debugger client's control.
|
||||
The result is Evidence with an initial state followed by one record per
|
||||
declared action. Each step reports action status, elapsed time, sanitized URLs,
|
||||
event bounds, and independently typed capture states for screenshot, DOM,
|
||||
accessibility, URL, history, and storage. Console, page-error, network,
|
||||
accessibility, URL, history, and storage. Actions after a failed or cancelled
|
||||
action are not run and are recorded as `cancelled` with a not-executed error.
|
||||
Console, page-error, network,
|
||||
WebSocket, frame, worker, popup, and cancelled-download events are attributed to
|
||||
steps. Retained events and requested artifacts are returned inline; omitted
|
||||
events are counted by kind and identify affected network transactions.
|
||||
|
||||
@@ -116,7 +116,9 @@ The CLI and MCP surfaces accept the same schema. For a JSON request file:
|
||||
rea capture-electron-scenario scenario.json --json
|
||||
```
|
||||
|
||||
The result records action status and targets, correlated app/window/WebContents,
|
||||
The result records one status and target per declared action; actions after a
|
||||
failed or cancelled action are not run and are recorded as `cancelled` with a
|
||||
not-executed error. It also records correlated app/window/WebContents,
|
||||
preload, session, navigation, shell, permission, popup, download, protocol,
|
||||
native-addon, process, and IPC timeline events. IPC channels and argument-shape
|
||||
metadata retain their observed lengths; payload values are never retained.
|
||||
|
||||
@@ -362,6 +362,9 @@ describe("PlaywrightBrowserScenarioProvider", () => {
|
||||
expect(result.value.steps[1]?.error).toBe(
|
||||
"fixture [REDACTED] action failure",
|
||||
);
|
||||
expect(result.value.steps[2]?.error).toBe(
|
||||
"not executed after an earlier action failure",
|
||||
);
|
||||
expect(session.performCalls).toBe(1);
|
||||
expect(session.closeCalls).toBe(1);
|
||||
expect(result.value.completeness.equality_eligible).toBe(false);
|
||||
@@ -713,7 +716,7 @@ describe("PlaywrightBrowserScenarioProvider request cancellation", () => {
|
||||
const provider = new PlaywrightBrowserScenarioProvider(() =>
|
||||
Promise.resolve(session),
|
||||
);
|
||||
const result = await provider.captureScenario(scenario(), {
|
||||
const result = await provider.captureScenario(scenario({ actions: 2 }), {
|
||||
signal: controller.signal,
|
||||
});
|
||||
expect(result).toMatchObject({
|
||||
@@ -724,12 +727,20 @@ describe("PlaywrightBrowserScenarioProvider request cancellation", () => {
|
||||
kind: "browser-scenario-observation",
|
||||
capture: {
|
||||
browser: { cleanup: "terminated-owned-process" },
|
||||
steps: [{ status: "completed" }, { status: "cancelled" }],
|
||||
steps: [
|
||||
{ status: "completed" },
|
||||
{ status: "cancelled" },
|
||||
{
|
||||
status: "cancelled",
|
||||
error: "not executed after scenario request cancellation",
|
||||
},
|
||||
],
|
||||
},
|
||||
},
|
||||
},
|
||||
});
|
||||
if (result.ok) throw new Error("Expected active scenario cancellation");
|
||||
expect(session.performCalls).toBe(1);
|
||||
expect(result.error.cause).toBe(cancellationReason);
|
||||
expect(session.closeCalls).toBe(1);
|
||||
});
|
||||
|
||||
@@ -39,6 +39,10 @@ import type {
|
||||
BrowserScenarioSessionOpener,
|
||||
BrowserScenarioSessionPort,
|
||||
} from "./BrowserScenarioSessionPort.js";
|
||||
import {
|
||||
notExecutedActionError,
|
||||
type ScenarioActionStop,
|
||||
} from "./ScenarioActionSequence.js";
|
||||
import {
|
||||
browserScenarioCleanupObservation,
|
||||
browserScenarioOperationFailure,
|
||||
@@ -161,14 +165,15 @@ const executeStep = async (input: {
|
||||
readonly scenario: BrowserScenario;
|
||||
readonly action: BrowserScenarioAction;
|
||||
readonly stepIndex: number;
|
||||
readonly priorFailure: boolean;
|
||||
readonly stoppedBy: ScenarioActionStop | undefined;
|
||||
readonly signal: AbortSignal | undefined;
|
||||
}): Promise<BrowserScenarioStep> => {
|
||||
const { session, scenario, action, stepIndex, priorFailure, signal } = input;
|
||||
const { session, scenario, action, stepIndex, stoppedBy, signal } = input;
|
||||
const beforeUrl = session.sanitizeUrl(session.currentUrl());
|
||||
const eventStart = session.nextEventSequence();
|
||||
const requested = requestedForStep(scenario, stepIndex);
|
||||
if (priorFailure)
|
||||
if (stoppedBy !== undefined) {
|
||||
const error = notExecutedActionError(stoppedBy);
|
||||
return createStep({
|
||||
stepIndex,
|
||||
stepId: action.step_id,
|
||||
@@ -177,14 +182,12 @@ const executeStep = async (input: {
|
||||
elapsedMs: 0,
|
||||
beforeUrl,
|
||||
afterUrl: beforeUrl,
|
||||
error: "not executed after an earlier action failure",
|
||||
error,
|
||||
eventStart,
|
||||
eventEnd: session.lastEventSequence(),
|
||||
artifacts: unavailableArtifacts(
|
||||
requested,
|
||||
"action was not executed after an earlier failure",
|
||||
),
|
||||
artifacts: unavailableArtifacts(requested, error),
|
||||
});
|
||||
}
|
||||
|
||||
const actionStartedAt = performance.now();
|
||||
session.setStep(stepIndex);
|
||||
@@ -270,18 +273,19 @@ const runScenario = async (
|
||||
signal: options.signal,
|
||||
}),
|
||||
);
|
||||
let failed = false;
|
||||
let stoppedBy: ScenarioActionStop | undefined;
|
||||
for (const [offset, action] of scenario.actions.entries()) {
|
||||
const step = await executeStep({
|
||||
session,
|
||||
scenario,
|
||||
action,
|
||||
stepIndex: offset + 1,
|
||||
priorFailure: failed,
|
||||
stoppedBy,
|
||||
signal: options.signal,
|
||||
});
|
||||
steps.push(step);
|
||||
failed ||= step.status !== "completed";
|
||||
if (stoppedBy === undefined && step.status !== "completed")
|
||||
stoppedBy = step.status;
|
||||
}
|
||||
} catch (cause: unknown) {
|
||||
operationOutcome = { ok: false, cause };
|
||||
|
||||
@@ -410,6 +410,46 @@ it("cancels a wait without window_index while the app has no window", async () =
|
||||
]);
|
||||
});
|
||||
|
||||
it("records actions after a cancelled action as not executed", async () => {
|
||||
const controller = new AbortController();
|
||||
const reload = vi.fn(async () => undefined);
|
||||
const page = {
|
||||
locator: () => ({
|
||||
click: () => {
|
||||
queueMicrotask(() => controller.abort());
|
||||
return new Promise<never>(() => undefined);
|
||||
},
|
||||
}),
|
||||
reload,
|
||||
};
|
||||
|
||||
const actions = await runElectronActions(
|
||||
{ windows: () => [page] } as unknown as ElectronApplication,
|
||||
electronActiveObservationInputSchema.parse({
|
||||
executable_path: "/opt/electron",
|
||||
application_path: "/opt/app/main.js",
|
||||
application_root: "/opt/app",
|
||||
actions: [
|
||||
{ step_id: "open", kind: "click", selector: "#open" },
|
||||
{ step_id: "reload", kind: "renderer-reload" },
|
||||
],
|
||||
}),
|
||||
{ signal: controller.signal },
|
||||
);
|
||||
|
||||
expect(reload).not.toHaveBeenCalled();
|
||||
expect(actions.map(({ step_id, status }) => [step_id, status])).toEqual([
|
||||
["open", "cancelled"],
|
||||
["reload", "cancelled"],
|
||||
]);
|
||||
expect(actions[1]).toMatchObject({
|
||||
window_index: 0,
|
||||
target: null,
|
||||
elapsed_ms: 0,
|
||||
error: "not executed after scenario request cancellation",
|
||||
});
|
||||
});
|
||||
|
||||
it("bounds a final state read that is still pending after cancellation", async () => {
|
||||
vi.useFakeTimers();
|
||||
const controller = new AbortController();
|
||||
|
||||
@@ -8,6 +8,10 @@ import {
|
||||
type ElectronActiveObservationInput,
|
||||
type ElectronActiveObservationResult,
|
||||
} from "../domain/javascript/electronActiveObservation.js";
|
||||
import {
|
||||
notExecutedActionError,
|
||||
type ScenarioActionStop,
|
||||
} from "./ScenarioActionSequence.js";
|
||||
|
||||
const OPERATION = "capture_electron_scenario" as const;
|
||||
const CANCELLED_STATE_READ_TIMEOUT_MS = 5_000;
|
||||
@@ -69,16 +73,33 @@ type ElectronApplicationState = Awaited<
|
||||
ReturnType<typeof readApplicationState>
|
||||
>;
|
||||
|
||||
/** Run explicit actions against provider-owned Electron windows. */
|
||||
/**
|
||||
* Run explicit actions against provider-owned Electron windows. Every declared
|
||||
* action has one result; actions after a failed or cancelled action are
|
||||
* recorded as not executed.
|
||||
*/
|
||||
export const runElectronActions = async (
|
||||
application: ElectronApplication,
|
||||
input: ElectronActiveObservationInput,
|
||||
options: ExecutionOptions,
|
||||
): Promise<ElectronActions> => {
|
||||
const actions: ElectronActions = [];
|
||||
let stoppedBy: ScenarioActionStop | undefined;
|
||||
for (const action of input.actions) {
|
||||
const actionStartedAt = performance.now();
|
||||
const windowIndex = actionWindowIndex(action);
|
||||
if (stoppedBy !== undefined) {
|
||||
actions.push({
|
||||
step_id: action.step_id,
|
||||
kind: action.kind,
|
||||
window_index: windowIndex,
|
||||
target: null,
|
||||
status: "cancelled",
|
||||
elapsed_ms: 0,
|
||||
error: notExecutedActionError(stoppedBy),
|
||||
});
|
||||
continue;
|
||||
}
|
||||
let selectedWindow: Page | undefined;
|
||||
try {
|
||||
if (options.signal?.aborted === true)
|
||||
@@ -119,20 +140,20 @@ export const runElectronActions = async (
|
||||
error: null,
|
||||
});
|
||||
} catch (cause: unknown) {
|
||||
stoppedBy = options.signal?.aborted ? "cancelled" : "failed";
|
||||
actions.push({
|
||||
step_id: action.step_id,
|
||||
kind: action.kind,
|
||||
window_index: windowIndex,
|
||||
target:
|
||||
selectedWindow === undefined ? null : `window:${String(windowIndex)}`,
|
||||
status: options.signal?.aborted ? "cancelled" : "failed",
|
||||
status: stoppedBy,
|
||||
elapsed_ms: Math.max(
|
||||
0,
|
||||
Math.round(performance.now() - actionStartedAt),
|
||||
),
|
||||
error: safeActionErrorMessage(cause),
|
||||
});
|
||||
break;
|
||||
}
|
||||
}
|
||||
return actions;
|
||||
|
||||
@@ -490,6 +490,80 @@ it("cancels a windowless wait action and cleans up the owned application", async
|
||||
expect(result.error.cause).toBe(reason);
|
||||
});
|
||||
|
||||
it("records actions after a failed action as not executed and keeps the capture", async () => {
|
||||
stubVerifiedElectronCleanup();
|
||||
const state = {
|
||||
windows: [],
|
||||
metrics: [],
|
||||
electronVersion: "44.0.0",
|
||||
hookSnapshot: emptyHookSnapshot(),
|
||||
};
|
||||
vi.spyOn(electronActions, "readApplicationState").mockResolvedValue(state);
|
||||
vi.spyOn(electronActions, "readFinalApplicationState").mockResolvedValue(
|
||||
state,
|
||||
);
|
||||
const click = vi.fn(async () => {
|
||||
throw new Error("strict mode violation");
|
||||
});
|
||||
const waitForTimeout = vi.fn(async () => undefined);
|
||||
const page = { locator: () => ({ click }), waitForTimeout };
|
||||
const application = {
|
||||
process: () => ({ pid: 424242 }),
|
||||
close: vi.fn(async () => undefined),
|
||||
windows: () => [page],
|
||||
firstWindow: async () => page,
|
||||
};
|
||||
|
||||
const result = await captureWithController(
|
||||
new AbortController(),
|
||||
application,
|
||||
[
|
||||
{ step_id: "fail", kind: "click", selector: "#duplicate" },
|
||||
{ step_id: "settle", kind: "wait", duration_ms: 0, window_index: 0 },
|
||||
{
|
||||
step_id: "link",
|
||||
kind: "deep-link",
|
||||
delivery: "open-url",
|
||||
url: "rea-test://open",
|
||||
},
|
||||
],
|
||||
);
|
||||
|
||||
if (!result.ok) throw result.error;
|
||||
expect(click).toHaveBeenCalledOnce();
|
||||
expect(waitForTimeout).not.toHaveBeenCalled();
|
||||
expect(application.close).toHaveBeenCalledOnce();
|
||||
expect(result.value.actions).toEqual([
|
||||
{
|
||||
step_id: "fail",
|
||||
kind: "click",
|
||||
window_index: 0,
|
||||
target: "window:0",
|
||||
status: "failed",
|
||||
elapsed_ms: expect.any(Number),
|
||||
error: "strict mode violation",
|
||||
},
|
||||
{
|
||||
step_id: "settle",
|
||||
kind: "wait",
|
||||
window_index: 0,
|
||||
target: null,
|
||||
status: "cancelled",
|
||||
elapsed_ms: 0,
|
||||
error: "not executed after an earlier action failure",
|
||||
},
|
||||
{
|
||||
step_id: "link",
|
||||
kind: "deep-link",
|
||||
window_index: null,
|
||||
target: null,
|
||||
status: "cancelled",
|
||||
elapsed_ms: 0,
|
||||
error: "not executed after an earlier action failure",
|
||||
},
|
||||
]);
|
||||
});
|
||||
|
||||
it("cancels a pending initial state read and cleans up the owned application", async () => {
|
||||
const cleanupSpies = stubVerifiedElectronCleanup();
|
||||
const controller = new AbortController();
|
||||
|
||||
@@ -0,0 +1,14 @@
|
||||
/** Status of the earlier action that stopped a declared scenario sequence. */
|
||||
export type ScenarioActionStop = "failed" | "cancelled";
|
||||
|
||||
/**
|
||||
* Explain a declared action recorded as `cancelled` without running because an
|
||||
* earlier action stopped the sequence. Scenario producers record every
|
||||
* remaining action so results account for the whole declared sequence.
|
||||
*/
|
||||
export const notExecutedActionError = (
|
||||
stoppedBy: ScenarioActionStop,
|
||||
): string =>
|
||||
stoppedBy === "failed"
|
||||
? "not executed after an earlier action failure"
|
||||
: "not executed after scenario request cancellation";
|
||||
Reference in New Issue
Block a user