ewilliams28/openclaw
1
0
Fork 0
mirror of https://github.com/openclaw/openclaw.git synced 2026-08-27 21:07:01 -06:00
Code Issues Packages Projects Releases Wiki Activity

fix(browser): cancel downloads after output save failures (#131048)

Browse Source
This commit is contained in:
Peter Steinberger
2026-08-27 10:28:08 -07:00
committed by GitHub
parent 1f148ebab5
commit 834368b7d4
5 changed files with 122 additions and 81 deletions
Download Patch File Download Diff File Expand all files Collapse all files
docs/cli/browser.md
+2
View File
@@ -275,6 +275,8 @@ openclaw browser dialog --dismiss --dialog-id d1
Managed Chrome profiles save ordinary click-triggered downloads into the OpenClaw downloads directory (`/tmp/openclaw/downloads` by default, or the configured temp root). Use `waitfordownload` or `download` when the agent needs to wait for a specific file and return its path; those explicit waiters own the next download. Uploads accept files from the OpenClaw temp uploads root and OpenClaw-managed inbound media, including `media://inbound/<id>` and sandbox-relative `media/inbound/<id>` references. Nested media refs, traversal, and arbitrary local paths are rejected.
If saving a download fails, OpenClaw requests cancellation of the transfer and reports the original save error. Correct the output path or filesystem problem before starting a new download.
When an action opens a modal dialog, the action response returns `blockedByDialog` with `browserState.dialogs.pending`; pass `--dialog-id` to answer it directly. Dialogs handled outside OpenClaw appear under `browserState.dialogs.recent`.
Batch actions:
extensions/browser/src/browser/pw-download-cancel.chromium.test.ts
+105 -81
View File
@@ -43,93 +43,117 @@ describe.runIf(runChromiumProof)("managed Chromium download cancellation", () =>
const cleanup: Array<() => Promise<void>> = [];
afterEach(async () => {
const errors: unknown[] = [];
for (const dispose of cleanup.splice(0).toReversed()) {
await dispose();
await dispose().catch((error: unknown) => errors.push(error));
}
if (errors.length) {
throw new AggregateError(errors, "Chromium download fixture cleanup failed");
}
});
it("cancels a download already being streamed without publishing its output", async () => {
const rootDir = tempDirs.make("openclaw-download-stream-cancel-");
cleanup.push(async () => await fs.rm(rootDir, { recursive: true, force: true }));
let closeDownloadResponse: (() => void) | undefined;
const downloadServer = createServer((request, response) => {
if (request.url === "/stream.bin") {
response.writeHead(200, {
"content-disposition": 'attachment; filename="stream.bin"',
"content-type": "application/octet-stream",
});
response.write("partially downloaded bytes");
closeDownloadResponse = () => response.destroy();
return;
it.each(["caller abort", "invalid output directory"])(
"cancels a streaming download after %s without publishing output",
async (failure) => {
const rootDir = tempDirs.make("openclaw-download-stream-cancel-");
cleanup.push(async () => await fs.rm(rootDir, { recursive: true, force: true }));
let closeDownloadResponse: (() => void) | undefined;
let responseClosed = false;
const downloadServer = createServer((request, response) => {
if (request.url === "/stream.bin") {
response.writeHead(200, {
"content-disposition": 'attachment; filename="stream.bin"',
"content-type": "application/octet-stream",
});
response.write("partially downloaded bytes");
response.once("close", () => {
responseClosed = true;
});
closeDownloadResponse = () => response.destroy();
return;
}
response.writeHead(200, { "content-type": "text/html; charset=utf-8" });
response.end('<a id="download" href="/stream.bin" download>Download</a>');
});
const downloadPort = await listen(downloadServer);
cleanup.push(async () => {
closeDownloadResponse?.();
await closeServer(downloadServer);
});
const cdpPort = await getFreePort();
const context = await getPlaywrightCore().chromium.launchPersistentContext(
path.join(rootDir, "profile"),
{
headless: true,
executablePath: process.env.PLAYWRIGHT_CHROMIUM_EXECUTABLE_PATH,
args: [`--remote-debugging-port=${cdpPort}`],
},
);
cleanup.push(async () => await context.close());
const page = context.pages()[0] ?? (await context.newPage());
await page.goto(`http://127.0.0.1:${downloadPort}/`);
const cdpUrl = `http://127.0.0.1:${cdpPort}`;
const targetId = await readTargetId(page);
cleanup.push(async () => await closePlaywrightBrowserConnection({ cdpUrl }));
cleanup.push(async () => closeDownloadResponse?.());
const controlledPage = await getPageForTargetId({ cdpUrl, targetId });
const saveStarted = createDeferred<void>();
let cancellationCount = 0;
controlledPage.once("download", (download) => {
const saveAs = download.saveAs.bind(download);
const cancel = download.cancel.bind(download);
download.saveAs = async (outputPath) => {
saveStarted.resolve();
await saveAs(outputPath);
};
download.cancel = async () => {
cancellationCount += 1;
await cancel();
};
});
const outputRoot = path.join(rootDir, "downloads");
if (failure === "invalid output directory") {
await fs.writeFile(outputRoot, "not a directory");
}
const outputPath = path.join(outputRoot, "cancelled.bin");
const controller = new AbortController();
const reason = new Error("streaming download aborted");
const capture = waitForDownloadViaPlaywright({
cdpUrl,
targetId,
path: outputPath,
rootDir: outputRoot,
timeoutMs: 5_000,
signal: controller.signal,
});
const outcome = capture.then(
() => "resolved" as const,
(error: unknown) => error,
);
await expect.poll(() => ensurePageState(controlledPage).downloadWaiterDepth).toBe(1);
await page.locator("#download").click();
if (failure === "caller abort") {
await Promise.race([saveStarted.promise, capture]);
controller.abort(reason);
await expect(outcome).resolves.toBe(reason);
await expect.poll(async () => await fs.readdir(outputRoot)).toEqual([]);
await expect(fs.access(outputPath)).rejects.toMatchObject({ code: "ENOENT" });
} else {
await expect(outcome).resolves.toMatchObject({
message: "Invalid path: must stay within output directory",
});
await expect(fs.readFile(outputRoot, "utf8")).resolves.toBe("not a directory");
}
response.writeHead(200, { "content-type": "text/html; charset=utf-8" });
response.end('<a id="download" href="/stream.bin" download>Download</a>');
});
const downloadPort = await listen(downloadServer);
cleanup.push(async () => {
closeDownloadResponse?.();
await closeServer(downloadServer);
});
const cdpPort = await getFreePort();
const context = await getPlaywrightCore().chromium.launchPersistentContext(
path.join(rootDir, "profile"),
{
headless: true,
executablePath: process.env.PLAYWRIGHT_CHROMIUM_EXECUTABLE_PATH,
args: [`--remote-debugging-port=${cdpPort}`],
},
);
cleanup.push(async () => await context.close());
const page = context.pages()[0] ?? (await context.newPage());
await page.goto(`http://127.0.0.1:${downloadPort}/`);
const cdpUrl = `http://127.0.0.1:${cdpPort}`;
const targetId = await readTargetId(page);
cleanup.push(async () => await closePlaywrightBrowserConnection({ cdpUrl }));
const controlledPage = await getPageForTargetId({ cdpUrl, targetId });
const saveStarted = createDeferred<void>();
let cancellationCount = 0;
controlledPage.once("download", (download) => {
const saveAs = download.saveAs.bind(download);
const cancel = download.cancel.bind(download);
download.saveAs = async (outputPath) => {
saveStarted.resolve();
await saveAs(outputPath);
};
download.cancel = async () => {
cancellationCount += 1;
await cancel();
};
});
const outputRoot = path.join(rootDir, "downloads");
const outputPath = path.join(outputRoot, "cancelled.bin");
const controller = new AbortController();
const reason = new Error("streaming download aborted");
const capture = waitForDownloadViaPlaywright({
cdpUrl,
targetId,
path: outputPath,
rootDir: outputRoot,
timeoutMs: 5_000,
signal: controller.signal,
});
const outcome = capture.then(
() => "resolved" as const,
(error: unknown) => error,
);
await expect.poll(() => ensurePageState(controlledPage).downloadWaiterDepth).toBe(1);
await page.locator("#download").click();
await saveStarted.promise;
controller.abort(reason);
expect(cancellationCount).toBe(1);
await expect(outcome).resolves.toBe(reason);
await expect.poll(async () => await fs.readdir(outputRoot)).toEqual([]);
await expect(fs.access(outputPath)).rejects.toMatchObject({ code: "ENOENT" });
expect(ensurePageState(controlledPage).downloadWaiterDepth).toBe(0);
}, 20_000);
expect.soft(cancellationCount).toBe(1);
await expect.poll(() => responseClosed).toBe(true);
expect(ensurePageState(controlledPage).downloadWaiterDepth).toBe(0);
},
20_000,
);
it("does not let a cancelled waiter capture and write a later download", async () => {
const rootDir = tempDirs.make("openclaw-download-cancel-");
extensions/browser/src/browser/pw-download-capture.ts
+7
View File
@@ -65,6 +65,13 @@ export async function saveBrowserDownload(
opts.signal?.throwIfAborted();
onReadyToPublish?.();
},
}).catch((error: unknown) => {
// Admission failures can belong to a superseded waiter. Only failed saves
// cancel here; an aborted capture already owns its cancellation.
if (!opts.signal?.aborted) {
void download.cancel?.().catch(() => {});
}
throw error;
});
return { ...candidate, path: savedPath };
}
extensions/browser/src/browser/pw-session.test.ts
+5
View File
@@ -657,15 +657,20 @@ describe("pw-session ensurePageState", () => {
ensurePageState(page);
const capture = beginActionDownloadCaptureOnPage(page);
const error = new Error("action download save failed");
const cancel = vi.fn(async () => {
throw new Error("browser disconnected during cancellation");
});
handlers.get("download")?.[0]?.({
suggestedFilename: () => "failed.txt",
saveAs: vi.fn(async () => {
throw error;
}),
cancel,
});
await expect(capture.drain()).rejects.toBe(error);
expect(cancel).toHaveBeenCalledOnce();
capture.dispose();
});
extensions/browser/src/browser/pw-tools-core.waits-next-download-saves-it.test.ts
+3
View File
@@ -358,6 +358,7 @@ describe("pw-tools-core", () => {
it("lets only the latest overlapping explicit waiter save the download", async () => {
const harness = createDownloadEventHarness();
const state = sessionMocks.ensurePageState();
const cancel = vi.fn(async () => {});
const saveAs = vi.fn(async (outPath: string) => {
await fs.writeFile(outPath, "latest-content", "utf8");
});
@@ -380,10 +381,12 @@ describe("pw-tools-core", () => {
url: () => "https://example.com/latest.bin",
suggestedFilename: () => "latest.bin",
saveAs,
cancel,
});
await expect(first).rejects.toThrow("superseded by another waiter");
await expect(latest).resolves.toMatchObject({ suggestedFilename: "latest.bin" });
expect(cancel).not.toHaveBeenCalled();
expect(saveAs).toHaveBeenCalledOnce();
expect(state.downloadWaiterDepth).toBe(0);
expect(harness.activeHandlerCount()).toBe(0);