Skip to content

Commit

Permalink
feat: implement detailed errors for evaluation (#10114)
Browse files Browse the repository at this point in the history
  • Loading branch information
jrandolf committed May 10, 2023
1 parent 75a5025 commit 317fa73
Show file tree
Hide file tree
Showing 19 changed files with 568 additions and 70 deletions.
1 change: 0 additions & 1 deletion docs/api/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,6 @@ sidebar_label: API
| [defaultArgs](./puppeteer.defaultargs.md) | |
| [devices](./puppeteer.devices.md) | |
| [errors](./puppeteer.errors.md) | |
| [EVALUATION_SCRIPT_URL](./puppeteer.evaluation_script_url.md) | |
| [executablePath](./puppeteer.executablepath.md) | |
| [KnownDevices](./puppeteer.knowndevices.md) | A list of devices to be used with [Page.emulate()](./puppeteer.page.emulate.md). |
| [launch](./puppeteer.launch.md) | |
Expand Down
11 changes: 0 additions & 11 deletions docs/api/puppeteer.evaluation_script_url.md

This file was deleted.

12 changes: 8 additions & 4 deletions packages/puppeteer-core/src/common/Coverage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,13 @@ import {Protocol} from 'devtools-protocol';
import {assert} from '../util/assert.js';

import {CDPSession} from './Connection.js';
import {EVALUATION_SCRIPT_URL} from './ExecutionContext.js';
import {addEventListener, debugError, PuppeteerEventListener} from './util.js';
import {removeEventListeners} from './util.js';
import {
addEventListener,
debugError,
PuppeteerEventListener,
PuppeteerURL,
removeEventListeners,
} from './util.js';

/**
* @internal
Expand Down Expand Up @@ -264,7 +268,7 @@ export class JSCoverage {
event: Protocol.Debugger.ScriptParsedEvent
): Promise<void> {
// Ignore puppeteer-injected scripts
if (event.url === EVALUATION_SCRIPT_URL) {
if (PuppeteerURL.isPuppeteerURL(event.url)) {
return;
}
// Ignore other anonymous scripts unless the reportAnonymousScripts option is true.
Expand Down
4 changes: 3 additions & 1 deletion packages/puppeteer-core/src/common/ElementHandle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ import {LazyArg} from './LazyArg.js';
import {CDPPage} from './Page.js';
import {ElementFor, EvaluateFuncWith, HandleFor, NodeFor} from './types.js';
import {KeyInput} from './USKeyboardLayout.js';
import {debugError, isString} from './util.js';
import {debugError, isString, withSourcePuppeteerURLIfNone} from './util.js';

const applyOffsetsToQuad = (
quad: Point[],
Expand Down Expand Up @@ -138,6 +138,7 @@ export class CDPElementHandle<
pageFunction: Func | string,
...args: Params
): Promise<Awaited<ReturnType<Func>>> {
pageFunction = withSourcePuppeteerURLIfNone(this.$eval.name, pageFunction);
const elementHandle = await this.$(selector);
if (!elementHandle) {
throw new Error(
Expand All @@ -161,6 +162,7 @@ export class CDPElementHandle<
pageFunction: Func | string,
...args: Params
): Promise<Awaited<ReturnType<Func>>> {
pageFunction = withSourcePuppeteerURLIfNone(this.$$eval.name, pageFunction);
const results = await this.$$(selector);
const elements = await this.evaluateHandle((_, ...elements) => {
return elements;
Expand Down
35 changes: 21 additions & 14 deletions packages/puppeteer-core/src/common/ExecutionContext.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,18 +32,20 @@ import {LazyArg} from './LazyArg.js';
import {scriptInjector} from './ScriptInjector.js';
import {EvaluateFunc, HandleFor} from './types.js';
import {
PuppeteerURL,
createEvaluationError,
createJSHandle,
getExceptionMessage,
getSourcePuppeteerURLIfAvailable,
isString,
valueFromRemoteObject,
} from './util.js';

/**
* @public
*/
export const EVALUATION_SCRIPT_URL = 'pptr://__puppeteer_evaluation_script__';
const SOURCE_URL_REGEX = /^[\040\t]*\/\/[@#] sourceURL=\s*(\S*?)\s*$/m;

const getSourceUrlComment = (url: string) => {
return `//# sourceURL=${url}`;
};

/**
* Represents a context for JavaScript execution.
*
Expand Down Expand Up @@ -270,14 +272,17 @@ export class ExecutionContext {
pageFunction: Func | string,
...args: Params
): Promise<HandleFor<Awaited<ReturnType<Func>>> | Awaited<ReturnType<Func>>> {
const suffix = `//# sourceURL=${EVALUATION_SCRIPT_URL}`;
const sourceUrlComment = getSourceUrlComment(
getSourcePuppeteerURLIfAvailable(pageFunction)?.toString() ??
PuppeteerURL.INTERNAL_URL
);

if (isString(pageFunction)) {
const contextId = this._contextId;
const expression = pageFunction;
const expressionWithSourceUrl = SOURCE_URL_REGEX.test(expression)
? expression
: expression + '\n' + suffix;
: `${expression}\n${sourceUrlComment}\n`;

const {exceptionDetails, result: remoteObject} = await this._client
.send('Runtime.evaluate', {
Expand All @@ -290,20 +295,24 @@ export class ExecutionContext {
.catch(rewriteError);

if (exceptionDetails) {
throw new Error(
'Evaluation failed: ' + getExceptionMessage(exceptionDetails)
);
throw createEvaluationError(exceptionDetails);
}

return returnByValue
? valueFromRemoteObject(remoteObject)
: createJSHandle(this, remoteObject);
}

const functionDeclaration = stringifyFunction(pageFunction);
const functionDeclarationWithSourceUrl = SOURCE_URL_REGEX.test(
functionDeclaration
)
? functionDeclaration
: `${functionDeclaration}\n${sourceUrlComment}\n`;
let callFunctionOnPromise;
try {
callFunctionOnPromise = this._client.send('Runtime.callFunctionOn', {
functionDeclaration: `${stringifyFunction(pageFunction)}\n${suffix}\n`,
functionDeclaration: functionDeclarationWithSourceUrl,
executionContextId: this._contextId,
arguments: await Promise.all(args.map(convertArgument.bind(this))),
returnByValue,
Expand All @@ -322,9 +331,7 @@ export class ExecutionContext {
const {exceptionDetails, result: remoteObject} =
await callFunctionOnPromise.catch(rewriteError);
if (exceptionDetails) {
throw new Error(
'Evaluation failed: ' + getExceptionMessage(exceptionDetails)
);
throw createEvaluationError(exceptionDetails);
}
return returnByValue
? valueFromRemoteObject(remoteObject)
Expand Down
12 changes: 11 additions & 1 deletion packages/puppeteer-core/src/common/Frame.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ import {MAIN_WORLD, PUPPETEER_WORLD} from './IsolatedWorlds.js';
import {LazyArg} from './LazyArg.js';
import {LifecycleWatcher, PuppeteerLifeCycleEvent} from './LifecycleWatcher.js';
import {EvaluateFunc, EvaluateFuncWith, HandleFor, NodeFor} from './types.js';
import {importFSPromises} from './util.js';
import {importFSPromises, withSourcePuppeteerURLIfNone} from './util.js';

/**
* @public
Expand Down Expand Up @@ -459,6 +459,10 @@ export class Frame {
pageFunction: Func | string,
...args: Params
): Promise<HandleFor<Awaited<ReturnType<Func>>>> {
pageFunction = withSourcePuppeteerURLIfNone(
this.evaluateHandle.name,
pageFunction
);
return this.worlds[MAIN_WORLD].evaluateHandle(pageFunction, ...args);
}

Expand All @@ -475,6 +479,10 @@ export class Frame {
pageFunction: Func | string,
...args: Params
): Promise<Awaited<ReturnType<Func>>> {
pageFunction = withSourcePuppeteerURLIfNone(
this.evaluate.name,
pageFunction
);
return this.worlds[MAIN_WORLD].evaluate(pageFunction, ...args);
}

Expand Down Expand Up @@ -536,6 +544,7 @@ export class Frame {
pageFunction: Func | string,
...args: Params
): Promise<Awaited<ReturnType<Func>>> {
pageFunction = withSourcePuppeteerURLIfNone(this.$eval.name, pageFunction);
return this.worlds[MAIN_WORLD].$eval(selector, pageFunction, ...args);
}

Expand Down Expand Up @@ -571,6 +580,7 @@ export class Frame {
pageFunction: Func | string,
...args: Params
): Promise<Awaited<ReturnType<Func>>> {
pageFunction = withSourcePuppeteerURLIfNone(this.$$eval.name, pageFunction);
return this.worlds[MAIN_WORLD].$$eval(selector, pageFunction, ...args);
}

Expand Down
6 changes: 3 additions & 3 deletions packages/puppeteer-core/src/common/FrameManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,15 @@ import {isErrorLike} from '../util/ErrorLike.js';
import {CDPSession, isTargetClosedError} from './Connection.js';
import {DeviceRequestPromptManager} from './DeviceRequestPrompt.js';
import {EventEmitter} from './EventEmitter.js';
import {EVALUATION_SCRIPT_URL, ExecutionContext} from './ExecutionContext.js';
import {ExecutionContext} from './ExecutionContext.js';
import {Frame} from './Frame.js';
import {FrameTree} from './FrameTree.js';
import {IsolatedWorld} from './IsolatedWorld.js';
import {MAIN_WORLD, PUPPETEER_WORLD} from './IsolatedWorlds.js';
import {NetworkManager} from './NetworkManager.js';
import {Target} from './Target.js';
import {TimeoutSettings} from './TimeoutSettings.js';
import {debugError} from './util.js';
import {debugError, PuppeteerURL} from './util.js';

const UTILITY_WORLD_NAME = '__puppeteer_utility_world__';

Expand Down Expand Up @@ -349,7 +349,7 @@ export class FrameManager extends EventEmitter {
}

await session.send('Page.addScriptToEvaluateOnNewDocument', {
source: `//# sourceURL=${EVALUATION_SCRIPT_URL}`,
source: `//# sourceURL=${PuppeteerURL.INTERNAL_URL}`,
worldName: name,
});

Expand Down
11 changes: 11 additions & 0 deletions packages/puppeteer-core/src/common/IsolatedWorld.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ import {
createJSHandle,
debugError,
setPageContent,
withSourcePuppeteerURLIfNone,
} from './util.js';
import {TaskManager, WaitTask} from './WaitTask.js';

Expand Down Expand Up @@ -183,6 +184,10 @@ export class IsolatedWorld {
pageFunction: Func | string,
...args: Params
): Promise<HandleFor<Awaited<ReturnType<Func>>>> {
pageFunction = withSourcePuppeteerURLIfNone(
this.evaluateHandle.name,
pageFunction
);
const context = await this.executionContext();
return context.evaluateHandle(pageFunction, ...args);
}
Expand All @@ -194,6 +199,10 @@ export class IsolatedWorld {
pageFunction: Func | string,
...args: Params
): Promise<Awaited<ReturnType<Func>>> {
pageFunction = withSourcePuppeteerURLIfNone(
this.evaluate.name,
pageFunction
);
const context = await this.executionContext();
return context.evaluate(pageFunction, ...args);
}
Expand Down Expand Up @@ -240,6 +249,7 @@ export class IsolatedWorld {
pageFunction: Func | string,
...args: Params
): Promise<Awaited<ReturnType<Func>>> {
pageFunction = withSourcePuppeteerURLIfNone(this.$eval.name, pageFunction);
const document = await this.document();
return document.$eval(selector, pageFunction, ...args);
}
Expand All @@ -256,6 +266,7 @@ export class IsolatedWorld {
pageFunction: Func | string,
...args: Params
): Promise<Awaited<ReturnType<Func>>> {
pageFunction = withSourcePuppeteerURLIfNone(this.$$eval.name, pageFunction);
const document = await this.document();
return document.$$eval(selector, pageFunction, ...args);
}
Expand Down
15 changes: 14 additions & 1 deletion packages/puppeteer-core/src/common/JSHandle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,12 @@ import {CDPSession} from './Connection.js';
import type {CDPElementHandle} from './ElementHandle.js';
import {ExecutionContext} from './ExecutionContext.js';
import {EvaluateFuncWith, HandleFor, HandleOr} from './types.js';
import {createJSHandle, releaseObject, valueFromRemoteObject} from './util.js';
import {
createJSHandle,
releaseObject,
valueFromRemoteObject,
withSourcePuppeteerURLIfNone,
} from './util.js';

declare const __JSHandleSymbol: unique symbol;

Expand Down Expand Up @@ -71,6 +76,10 @@ export class CDPJSHandle<T = unknown> extends JSHandle<T> {
pageFunction: Func | string,
...args: Params
): Promise<Awaited<ReturnType<Func>>> {
pageFunction = withSourcePuppeteerURLIfNone(
this.evaluate.name,
pageFunction
);
return await this.executionContext().evaluate(pageFunction, this, ...args);
}

Expand All @@ -84,6 +93,10 @@ export class CDPJSHandle<T = unknown> extends JSHandle<T> {
pageFunction: Func | string,
...args: Params
): Promise<HandleFor<Awaited<ReturnType<Func>>>> {
pageFunction = withSourcePuppeteerURLIfNone(
this.evaluateHandle.name,
pageFunction
);
return await this.executionContext().evaluateHandle(
pageFunction,
this,
Expand Down
18 changes: 13 additions & 5 deletions packages/puppeteer-core/src/common/Page.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,10 +85,10 @@ import {
NodeFor,
} from './types.js';
import {
createClientError,
createJSHandle,
debugError,
evaluationString,
getExceptionMessage,
getReadableAsBuffer,
getReadableFromProtocolStream,
isString,
Expand All @@ -97,6 +97,7 @@ import {
valueFromRemoteObject,
waitForEvent,
waitWithTimeout,
withSourcePuppeteerURLIfNone,
} from './util.js';
import {WebWorker} from './WebWorker.js';

Expand Down Expand Up @@ -518,6 +519,10 @@ export class CDPPage extends Page {
pageFunction: Func | string,
...args: Params
): Promise<HandleFor<Awaited<ReturnType<Func>>>> {
pageFunction = withSourcePuppeteerURLIfNone(
this.evaluateHandle.name,
pageFunction
);
const context = await this.mainFrame().executionContext();
return context.evaluateHandle(pageFunction, ...args);
}
Expand Down Expand Up @@ -549,6 +554,7 @@ export class CDPPage extends Page {
pageFunction: Func | string,
...args: Params
): Promise<Awaited<ReturnType<Func>>> {
pageFunction = withSourcePuppeteerURLIfNone(this.$eval.name, pageFunction);
return this.mainFrame().$eval(selector, pageFunction, ...args);
}

Expand All @@ -564,6 +570,7 @@ export class CDPPage extends Page {
pageFunction: Func | string,
...args: Params
): Promise<Awaited<ReturnType<Func>>> {
pageFunction = withSourcePuppeteerURLIfNone(this.$$eval.name, pageFunction);
return this.mainFrame().$$eval(selector, pageFunction, ...args);
}

Expand Down Expand Up @@ -735,10 +742,7 @@ export class CDPPage extends Page {
}

#handleException(exceptionDetails: Protocol.Runtime.ExceptionDetails): void {
const message = getExceptionMessage(exceptionDetails);
const err = new Error(message);
err.stack = ''; // Don't report clientside error with a node stack attached
this.emit(PageEmittedEvents.PageError, err);
this.emit(PageEmittedEvents.PageError, createClientError(exceptionDetails));
}

async #onConsoleAPI(
Expand Down Expand Up @@ -1257,6 +1261,10 @@ export class CDPPage extends Page {
pageFunction: Func | string,
...args: Params
): Promise<Awaited<ReturnType<Func>>> {
pageFunction = withSourcePuppeteerURLIfNone(
this.evaluate.name,
pageFunction
);
return this.#frameManager.mainFrame().evaluate(pageFunction, ...args);
}

Expand Down
Loading

0 comments on commit 317fa73

Please sign in to comment.