github-featured/chromedevtools--chrome-devtools-mcp
2
0
Fork 0
Code Issues Pull Requests Deployments Models Agent Notes Packages Projects Releases Wiki Activity

refactor: introduce Explicit resouce managent (using) (#2443)

Browse Source 
Re-export the Puppeteer helpers as we have tested those.
This should also be a small performance improvement as we don't await
any of the disposals.
Will also fix memory leaking for some methods.
With the added benefit of make the code look cleaner (no try/catch.)

Also Fixes
https://github.com/ChromeDevTools/chrome-devtools-mcp/issues/2440,
Closes https://github.com/ChromeDevTools/chrome-devtools-mcp/pull/2441
This commit is contained in:
Nikolay Vitkov
2026-07-30 21:21:23 +02:00
committed by GitHub
parent 775ea2d3a3
commit 5b25370c9c
8 changed files with 179 additions and 189 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/McpPage.ts
+7 -8
View File
@@ -4,6 +4,8 @@
* SPDX-License-Identifier: Apache-2.0
*/
import {DisposableStack} from './third_party/index.js';
export function replaceHtmlElementsWithUids(schema: JSONSchema7Definition) {
if (typeof schema === 'boolean') {
return;
@@ -225,7 +227,7 @@ export class McpPage implements ContextPage {
async getToolGroups(): Promise<ToolGroups> {
// Check if there is a `devtoolstooldiscovery` event listener
const windowHandle = await this.pptrPage.evaluateHandle(() => window);
using windowHandle = await this.pptrPage.evaluateHandle(() => window);
// @ts-expect-error internal API
const client = this.pptrPage._client();
const {listeners}: {listeners: Protocol.DOMDebugger.EventListener[]} =
@@ -560,17 +562,14 @@ export class McpPage implements ContextPage {
}
if (elementHandles.length) {
const oldHandles = [...this.extraHandles];
using stack = new DisposableStack();
for (const handle of elementHandles) {
stack.use(handle);
}
this.textSnapshot = await TextSnapshot.create(this, {
extraHandles: elementHandles,
});
response.includeSnapshot();
for (const handle of oldHandles) {
await handle
.dispose()
.catch(e => logger?.('Failed to dispose old handle', e));
}
}
const cdpElementIds = await Promise.all(
src/TextSnapshot.ts
+4 -3
View File
@@ -4,6 +4,7 @@
* SPDX-License-Identifier: Apache-2.0
*/
import {DisposableStack} from './third_party/index.js';
import type {McpPage} from './McpPage.js';
import type {
Protocol,
@@ -221,11 +222,13 @@ export class TextSnapshot {
handle: ElementHandle,
): Promise<TextSnapshotNode | null> => {
let ancestorHandle = await handle.evaluateHandle(el => el.parentElement);
using stack = new DisposableStack();
while (ancestorHandle) {
stack.use(ancestorHandle);
const ancestorElement = ancestorHandle.asElement();
if (!ancestorElement) {
await ancestorHandle.dispose();
return null;
}
@@ -235,7 +238,6 @@ export class TextSnapshot {
.values()
.find(node => node.backendNodeId === ancestorBackendId);
if (ancestorNode) {
await ancestorHandle.dispose();
return ancestorNode;
}
}
@@ -243,7 +245,6 @@ export class TextSnapshot {
const nextHandle = await ancestorElement.evaluateHandle(
el => el.parentElement,
);
await ancestorHandle.dispose();
ancestorHandle = nextHandle;
}
return null;
src/WaitForHelper.ts
+2 -3
View File
@@ -45,7 +45,7 @@ export class WaitForHelper {
// Bound the setup evaluation against the stable-DOM timeout. Without this
// cap a paused renderer (e.g. an open dialog) would make evaluateHandle
// hang until protocolTimeout (default 180s) while the tool mutex is held.
const stableDomObserver = await Promise.race([
using stableDomObserver = await Promise.race([
this.#page.evaluateHandle(timeout => {
let timeoutId: ReturnType<typeof setTimeout>;
function callback() {
@@ -71,7 +71,7 @@ export class WaitForHelper {
return domObserver;
}, this.#stableDomFor),
this.timeout(this.#stableDomTimeout),
this.timeout(this.#stableDomTimeout) as Promise<undefined>,
]).catch(() => undefined);
if (!stableDomObserver) {
@@ -84,7 +84,6 @@ export class WaitForHelper {
observer.observer.disconnect();
observer.resolver.resolve();
});
await stableDomObserver.dispose();
} catch {
// Ignored cleanup errors
}
src/third_party/index.ts
Vendored
+4
View File
@@ -50,6 +50,10 @@ export type {CdpWebWorker} from 'puppeteer-core/internal/cdp/WebWorker.js';
export type {Realm} from 'puppeteer-core/internal/api/Realm.js';
export type {JSONSchema7, JSONSchema7Definition} from 'json-schema';
export {Mutex} from 'puppeteer-core/internal/util/Mutex.js';
export {
DisposableStack,
AsyncDisposableStack,
} from 'puppeteer-core/internal/util/disposable.js';
export {
resolveDefaultUserDataDir,
detectBrowserPlatform,
src/tools/input.ts
+56 -80
View File
@@ -44,7 +44,7 @@ function handleActionError(error: unknown, uid: string) {
}
async function selectNativeSelectOption(handle: ElementHandle<Element>) {
const selectHandle = await handle.evaluateHandle(node => {
using selectHandle = await handle.evaluateHandle(node => {
if (!(node instanceof HTMLOptionElement)) {
return null;
}
@@ -64,26 +64,21 @@ async function selectNativeSelectOption(handle: ElementHandle<Element>) {
return select;
});
try {
const select = selectHandle.asElement() as ElementHandle<Element> | null;
if (!select) {
return false;
}
const valueHandle = await handle.getProperty('value');
try {
const value = await valueHandle.jsonValue();
if (typeof value !== 'string') {
return false;
}
await select.asLocator().fill(value);
} finally {
void valueHandle.dispose();
}
return true;
} finally {
void selectHandle.dispose();
using select = selectHandle.asElement() as ElementHandle<Element> | null;
if (!select) {
return false;
}
using valueHandle = await handle.getProperty('value');
const value = await valueHandle.jsonValue();
if (typeof value !== 'string') {
return false;
}
await select.asLocator().fill(value);
return true;
}
export const click = definePageTool({
@@ -106,7 +101,7 @@ export const click = definePageTool({
verifyFilesSchema: [],
handler: async (request, response) => {
const uid = request.params.uid;
const handle = await request.page.getElementByUid(uid);
using handle = await request.page.getElementByUid(uid);
const aXNode = request.page.getAXNodeByUid(uid);
const shouldSelectNativeOption =
!request.params.dblClick && aXNode?.role === 'option';
@@ -134,8 +129,6 @@ export const click = definePageTool({
}
} catch (error) {
handleActionError(error, uid);
} finally {
void handle.dispose();
}
},
});
@@ -194,7 +187,7 @@ export const hover = definePageTool({
verifyFilesSchema: [],
handler: async (request, response) => {
const uid = request.params.uid;
const handle = await request.page.getElementByUid(uid);
using handle = await request.page.getElementByUid(uid);
try {
const result = await request.page.waitForEventsAfterAction(async () => {
await handle.asLocator().hover();
@@ -206,8 +199,6 @@ export const hover = definePageTool({
}
} catch (error) {
handleActionError(error, uid);
} finally {
void handle.dispose();
}
},
});
@@ -225,22 +216,16 @@ async function selectOption(
for (const child of aXNode.children) {
if (child.role === 'option' && child.name === value && child.value) {
optionFound = true;
const childHandle = await child.elementHandle();
using childHandle = await child.elementHandle();
if (childHandle) {
try {
const childValueHandle = await childHandle.getProperty('value');
try {
const childValue = await childValueHandle.jsonValue();
if (childValue) {
await handle.asLocator().fill(childValue.toString());
}
} finally {
void childValueHandle.dispose();
}
break;
} finally {
void childHandle.dispose();
using childValueHandle = await childHandle.getProperty('value');
const childValue = await childValueHandle.jsonValue();
if (childValue) {
await handle.asLocator().fill(childValue.toString());
}
break;
}
}
}
@@ -259,7 +244,7 @@ async function fillFormElement(
context: McpContext,
page: ContextPage,
) {
const handle = await page.getElementByUid(uid);
using handle = await page.getElementByUid(uid);
try {
const aXNode = page.getAXNodeByUid(uid);
// We assume that combobox needs to be handled as select if it has
@@ -293,8 +278,6 @@ async function fillFormElement(
}
} catch (error) {
handleActionError(error, uid);
} finally {
void handle.dispose();
}
}
@@ -383,24 +366,20 @@ export const drag = definePageTool({
blockedByDialog: true,
verifyFilesSchema: [],
handler: async (request, response) => {
const fromHandle = await request.page.getElementByUid(
using fromHandle = await request.page.getElementByUid(
request.params.from_uid,
);
const toHandle = await request.page.getElementByUid(request.params.to_uid);
try {
const result = await request.page.waitForEventsAfterAction(async () => {
await fromHandle.drag(toHandle);
await new Promise(resolve => setTimeout(resolve, 50));
await toHandle.drop(fromHandle);
});
response.appendResponseLine(`Successfully dragged an element`);
response.attachWaitForResult(result);
if (request.params.includeSnapshot) {
response.includeSnapshot();
}
} finally {
void fromHandle.dispose();
void toHandle.dispose();
using toHandle = await request.page.getElementByUid(request.params.to_uid);
const result = await request.page.waitForEventsAfterAction(async () => {
await fromHandle.drag(toHandle);
await new Promise(resolve => setTimeout(resolve, 50));
await toHandle.drop(fromHandle);
});
response.appendResponseLine(`Successfully dragged an element`);
response.attachWaitForResult(result);
if (request.params.includeSnapshot) {
response.includeSnapshot();
}
},
});
@@ -471,35 +450,32 @@ export const uploadFile = definePageTool({
verifyFilesSchema: ['filePath'],
handler: async (request, response) => {
const {uid, filePath} = request.params;
const handle = (await request.page.getElementByUid(
using handle = (await request.page.getElementByUid(
uid,
)) as ElementHandle<HTMLInputElement>;
try {
await handle.uploadFile(filePath);
} catch {
// Some sites use a proxy element to trigger file upload instead of
// a type=file element. In this case, we want to default to
// Page.waitForFileChooser() and upload the file this way.
try {
await handle.uploadFile(filePath);
const [fileChooser] = await Promise.all([
request.page.pptrPage.waitForFileChooser({timeout: 3000}),
handle.asLocator().click(),
]);
await fileChooser.accept([filePath]);
} catch {
// Some sites use a proxy element to trigger file upload instead of
// a type=file element. In this case, we want to default to
// Page.waitForFileChooser() and upload the file this way.
try {
const [fileChooser] = await Promise.all([
request.page.pptrPage.waitForFileChooser({timeout: 3000}),
handle.asLocator().click(),
]);
await fileChooser.accept([filePath]);
} catch {
throw new Error(
`Failed to upload file. The element could not accept the file directly, and clicking it did not trigger a file chooser.`,
);
}
throw new Error(
`Failed to upload file. The element could not accept the file directly, and clicking it did not trigger a file chooser.`,
);
}
if (request.params.includeSnapshot) {
response.includeSnapshot();
}
response.appendResponseLine(`File uploaded from ${filePath}.`);
} finally {
void handle.dispose();
}
if (request.params.includeSnapshot) {
response.includeSnapshot();
}
response.appendResponseLine(`File uploaded from ${filePath}.`);
},
});
src/tools/screenshot.ts
+43 -46
View File
@@ -145,7 +145,7 @@ export const screenshot = definePageTool(args => {
}
const page = request.page.pptrPage;
const element = request.params.uid
using element = request.params.uid
? await request.page.getElementByUid(request.params.uid)
: undefined;
@@ -160,53 +160,50 @@ export const screenshot = definePageTool(args => {
// once the capture is done to avoid leaking a remote object for the life
// of the page's execution context.
let screenshot: Uint8Array;
try {
// Compute a downscale clip when --screenshot-max-width or
// --screenshot-max-height is set and the source exceeds either bound.
// The smaller scale factor wins so both bounds are respected while
// preserving aspect ratio.
let clip: ScreenshotClip | undefined;
if (
screenshotMaxWidth !== undefined ||
screenshotMaxHeight !== undefined
) {
const box = await getSourceBox(page, element, fullPage);
if (box) {
clip = computeDownscaleClip(
box,
screenshotMaxWidth,
screenshotMaxHeight,
);
}
}
if (clip) {
// page.screenshot with clip lets the CDP scale param downscale the
// capture for viewport, full-page and element shots alike. We rely on
// Puppeteer's default of captureBeyondViewport=true when a clip is
// present so element/full-page captures below the fold still work.
screenshot = await page.screenshot({
type: format,
quality,
optimizeForSpeed: true,
clip,
});
} else if (element) {
screenshot = await element.screenshot({
type: format,
quality,
optimizeForSpeed: true,
});
} else {
screenshot = await page.screenshot({
type: format,
fullPage,
quality,
optimizeForSpeed: true,
});
// Compute a downscale clip when --screenshot-max-width or
// --screenshot-max-height is set and the source exceeds either bound.
// The smaller scale factor wins so both bounds are respected while
// preserving aspect ratio.
let clip: ScreenshotClip | undefined;
if (
screenshotMaxWidth !== undefined ||
screenshotMaxHeight !== undefined
) {
const box = await getSourceBox(page, element, fullPage);
if (box) {
clip = computeDownscaleClip(
box,
screenshotMaxWidth,
screenshotMaxHeight,
);
}
} finally {
void element?.dispose();
}
if (clip) {
// page.screenshot with clip lets the CDP scale param downscale the
// capture for viewport, full-page and element shots alike. We rely on
// Puppeteer's default of captureBeyondViewport=true when a clip is
// present so element/full-page captures below the fold still work.
screenshot = await page.screenshot({
type: format,
quality,
optimizeForSpeed: true,
clip,
});
} else if (element) {
screenshot = await element.screenshot({
type: format,
quality,
optimizeForSpeed: true,
});
} else {
screenshot = await page.screenshot({
type: format,
fullPage,
quality,
optimizeForSpeed: true,
});
}
if (request.params.uid) {
src/tools/script.ts
+45 -49
View File
@@ -4,7 +4,7 @@
* SPDX-License-Identifier: Apache-2.0
*/
import {zod} from '../third_party/index.js';
import {DisposableStack, zod} from '../third_party/index.js';
import type {Frame, JSHandle, Page, WebWorker} from '../third_party/index.js';
import type {ExtensionServiceWorker} from '../types.js';
@@ -110,29 +110,28 @@ Example with arguments: \`(el) => el.innerText\`
const page: Page = mcpPage.pptrPage;
const args: Array<JSHandle<unknown>> = [];
try {
const frames = new Set<Frame>();
for (const uid of uidArgs ?? []) {
const handle = await mcpPage.getElementByUid(uid);
frames.add(handle.frame);
args.push(handle);
}
using stack = new DisposableStack();
const evaluatable = await getPageOrFrame(page, frames);
const result = await mcpPage.waitForEventsAfterAction(
async () => {
await performEvaluation(evaluatable, fnString, args, response, {
filePath,
context,
});
},
{handleDialog: dialogAction ?? 'accept'},
);
response.attachWaitForResult(result);
} finally {
void Promise.allSettled(args.map(arg => arg.dispose()));
const frames = new Set<Frame>();
for (const uid of uidArgs ?? []) {
const handle = await mcpPage.getElementByUid(uid);
frames.add(handle.frame);
stack.use(handle);
args.push(handle);
}
const evaluatable = await getPageOrFrame(page, frames);
const result = await mcpPage.waitForEventsAfterAction(
async () => {
await performEvaluation(evaluatable, fnString, args, response, {
filePath,
context,
});
},
{handleDialog: dialogAction ?? 'accept'},
);
response.attachWaitForResult(result);
},
};
});
@@ -144,34 +143,31 @@ const performEvaluation = async (
response: Response,
options?: {filePath: string; context: Context},
) => {
const fn = await evaluatable.evaluateHandle(`(${fnString})`);
try {
const result = await evaluatable.evaluate(
async (fn, ...args) => {
// @ts-expect-error no types for function fn
return JSON.stringify(await fn(...args));
},
fn,
...args,
using fn = await evaluatable.evaluateHandle(`(${fnString})`);
const result = await evaluatable.evaluate(
async (fn, ...args) => {
// @ts-expect-error no types for function fn
return JSON.stringify(await fn(...args));
},
fn,
...args,
);
if (options?.filePath) {
const data = new TextEncoder().encode(result ?? 'undefined');
const {filename} = await options.context.saveFile(
data,
options.filePath,
'.json',
);
if (options?.filePath) {
const data = new TextEncoder().encode(result ?? 'undefined');
const {filename} = await options.context.saveFile(
data,
options.filePath,
'.json',
);
response.appendResponseLine(
`Script ran on page. Output saved to ${filename}.`,
);
} else {
response.appendResponseLine('Script ran on page and returned:');
response.appendResponseLine('```json');
response.appendResponseLine(`${result}`);
response.appendResponseLine('```');
}
} finally {
void fn.dispose();
response.appendResponseLine(
`Script ran on page. Output saved to ${filename}.`,
);
} else {
response.appendResponseLine('Script ran on page and returned:');
response.appendResponseLine('```json');
response.appendResponseLine(`${result}`);
response.appendResponseLine('```');
}
};
tests/McpPage.test.ts
+18
View File
@@ -9,6 +9,7 @@ import {describe, it} from 'node:test';
import {replaceHtmlElementsWithUids} from '../src/McpPage.js';
import type {JSONSchema7Definition} from '../src/third_party/index.js';
import {withMcpContext} from './utils.js';
describe('replaceHtmlElementsWithUids', () => {
it('does nothing for boolean schemas', () => {
@@ -257,3 +258,20 @@ describe('replaceHtmlElementsWithUids', () => {
}
});
});
describe('McpPage', () => {
it('creates a handle on the page and disposes it as such', async () => {
await withMcpContext(async (response, context) => {
const page = context.getSelectedMcpPage().pptrPage;
using handle = await page.evaluateHandle('new Set()');
{
using _ = handle;
}
// @ts-expect-error Internal Puppeteer API
assert.ok(handle.disposed);
});
});
});