From 4d359906a44e4ddd5ec54a523cfd9076048d3433 Mon Sep 17 00:00:00 2001 From: jrandolf <101637635+jrandolf@users.noreply.github.com> Date: Fri, 10 Jun 2022 15:27:42 +0200 Subject: [PATCH] fix: use error-like (#8504) --- .gitignore | 2 +- src/common/BrowserConnector.ts | 4 ++-- src/common/FrameManager.ts | 6 +++--- src/common/Page.ts | 7 ++++--- src/common/Tracing.ts | 4 ++-- src/common/helper.ts | 20 +++++++++++++++++++- src/node/BrowserRunner.ts | 9 +++++---- test/mocha-utils.ts | 18 +++++++++--------- 8 files changed, 45 insertions(+), 25 deletions(-) diff --git a/.gitignore b/.gitignore index 0142fe48e125b..58dd057714da1 100644 --- a/.gitignore +++ b/.gitignore @@ -14,7 +14,7 @@ test-ts-types/**/dist/ package-lock.json yarn.lock /node6 -/lib +lib test/coverage.json temp/ new-docs/ diff --git a/src/common/BrowserConnector.ts b/src/common/BrowserConnector.ts index d9ede04f2ec5c..c2ce220fd78fe 100644 --- a/src/common/BrowserConnector.ts +++ b/src/common/BrowserConnector.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import { debugError } from '../common/helper.js'; +import { debugError, isErrorLike } from '../common/helper.js'; import { isNode } from '../environment.js'; import { assert } from './assert.js'; import { @@ -138,7 +138,7 @@ async function getWSEndpoint(browserURL: string): Promise { const data = await result.json(); return data.webSocketDebuggerUrl; } catch (error) { - if (error instanceof Error) { + if (isErrorLike(error)) { error.message = `Failed to fetch browser webSocket URL from ${endpointURL}: ` + error.message; diff --git a/src/common/FrameManager.ts b/src/common/FrameManager.ts index 68464351c9aa5..833568efd7a80 100644 --- a/src/common/FrameManager.ts +++ b/src/common/FrameManager.ts @@ -16,7 +16,7 @@ import { EventEmitter } from './EventEmitter.js'; import { assert } from './assert.js'; -import { helper, debugError } from './helper.js'; +import { helper, debugError, isErrorLike } from './helper.js'; import { ExecutionContext, EVALUATION_SCRIPT_URL } from './ExecutionContext.js'; import { LifecycleWatcher, @@ -163,7 +163,7 @@ export class FrameManager extends EventEmitter { } catch (error) { // The target might have been closed before the initialization finished. if ( - error instanceof Error && + isErrorLike(error) && (error.message.includes('Target closed') || error.message.includes('Session closed')) ) { @@ -226,7 +226,7 @@ export class FrameManager extends EventEmitter { ? new Error(`${response.errorText} at ${url}`) : null; } catch (error) { - if (error instanceof Error) { + if (isErrorLike(error)) { return error; } throw error; diff --git a/src/common/Page.ts b/src/common/Page.ts index c01c6beaf0179..8aa815b0b6b94 100644 --- a/src/common/Page.ts +++ b/src/common/Page.ts @@ -43,7 +43,7 @@ import { FrameManager, FrameManagerEmittedEvents, } from './FrameManager.js'; -import { debugError, helper } from './helper.js'; +import { debugError, helper, isErrorLike } from './helper.js'; import { HTTPRequest } from './HTTPRequest.js'; import { HTTPResponse } from './HTTPResponse.js'; import { Keyboard, Mouse, MouseButton, Touchscreen } from './Input.js'; @@ -1578,7 +1578,7 @@ export class Page extends EventEmitter { const result = await pageBinding(...args); expression = helper.pageBindingDeliverResultString(name, seq, result); } catch (error) { - if (error instanceof Error) + if (isErrorLike(error)) expression = helper.pageBindingDeliverErrorString( name, seq, @@ -2360,8 +2360,9 @@ export class Page extends EventEmitter { timezoneId: timezoneId || '', }); } catch (error) { - if (error instanceof Error && error.message.includes('Invalid timezone')) + if (isErrorLike(error) && error.message.includes('Invalid timezone')) { throw new Error(`Invalid timezone ID: ${timezoneId}`); + } throw error; } } diff --git a/src/common/Tracing.ts b/src/common/Tracing.ts index bffbf1ab2ca1e..e8c4b1b9adc40 100644 --- a/src/common/Tracing.ts +++ b/src/common/Tracing.ts @@ -14,7 +14,7 @@ * limitations under the License. */ import { assert } from './assert.js'; -import { helper } from './helper.js'; +import { helper, isErrorLike } from './helper.js'; import { CDPSession } from './Connection.js'; /** @@ -122,7 +122,7 @@ export class Tracing { const buffer = await helper.getReadableAsBuffer(readable, this._path); resolve(buffer ?? undefined); } catch (error) { - if (error instanceof Error) { + if (isErrorLike(error)) { reject(error); } else { reject(new Error(`Unknown error: ${error}`)); diff --git a/src/common/helper.ts b/src/common/helper.ts index 08a57483b3660..da1508a8d7e67 100644 --- a/src/common/helper.ts +++ b/src/common/helper.ts @@ -165,7 +165,9 @@ async function waitForEvent( throw error; } ); - if (result instanceof Error) throw result; + if (isErrorLike(result)) { + throw result; + } return result; } @@ -378,6 +380,22 @@ async function getReadableFromProtocolStream( }); } +interface ErrorLike extends Error { + name: string; + message: string; +} + +export function isErrorLike(obj: unknown): obj is ErrorLike { + return obj instanceof Object && 'name' in obj && 'message' in obj; +} + +export function isErrnoException(obj: unknown): obj is NodeJS.ErrnoException { + return ( + isErrorLike(obj) && + ('errno' in obj || 'code' in obj || 'path' in obj || 'syscall' in obj) + ); +} + export const helper = { evaluationString, pageBindingInitString, diff --git a/src/node/BrowserRunner.ts b/src/node/BrowserRunner.ts index e2cc686add6a2..ea330c8f4cf30 100644 --- a/src/node/BrowserRunner.ts +++ b/src/node/BrowserRunner.ts @@ -28,6 +28,8 @@ import { helper, debugError, PuppeteerEventListener, + isErrorLike, + isErrnoException, } from '../common/helper.js'; import { LaunchOptions } from './LaunchOptions.js'; import { Connection } from '../common/Connection.js'; @@ -216,7 +218,7 @@ export class BrowserRunner { } catch (error) { throw new Error( `${PROCESS_ERROR_EXPLANATION}\nError cause: ${ - error instanceof Error ? error.stack : error + isErrorLike(error) ? error.stack : error }` ); } @@ -330,9 +332,8 @@ function pidExists(pid: number): boolean { try { return process.kill(pid, 0); } catch (error) { - if (error instanceof Error) { - const err = error as NodeJS.ErrnoException; - if (err.code && err.code === 'ESRCH') { + if (isErrnoException(error)) { + if (error.code && error.code === 'ESRCH') { return false; } } diff --git a/test/mocha-utils.ts b/test/mocha-utils.ts index 37190b940f689..88dfacf31b49f 100644 --- a/test/mocha-utils.ts +++ b/test/mocha-utils.ts @@ -14,24 +14,24 @@ * limitations under the License. */ -import { TestServer } from '../utils/testserver/index.js'; -import * as path from 'path'; +import Protocol from 'devtools-protocol'; +import expect from 'expect'; import * as fs from 'fs'; import * as os from 'os'; +import * as path from 'path'; +import rimraf from 'rimraf'; import sinon from 'sinon'; -import puppeteer from '../lib/cjs/puppeteer/puppeteer.js'; import { Browser, BrowserContext, } from '../lib/cjs/puppeteer/common/Browser.js'; +import { isErrorLike } from '../lib/cjs/puppeteer/common/helper.js'; import { Page } from '../lib/cjs/puppeteer/common/Page.js'; import { PuppeteerNode } from '../lib/cjs/puppeteer/node/Puppeteer.js'; -import utils from './utils.js'; -import rimraf from 'rimraf'; -import expect from 'expect'; - +import puppeteer from '../lib/cjs/puppeteer/puppeteer.js'; +import { TestServer } from '../utils/testserver/index.js'; import { trackCoverage } from './coverage-utils.js'; -import Protocol from 'devtools-protocol'; +import utils from './utils.js'; const setupServer = async () => { const assetsPath = path.join(__dirname, 'assets'); @@ -73,7 +73,7 @@ let extraLaunchOptions = {}; try { extraLaunchOptions = JSON.parse(process.env['EXTRA_LAUNCH_OPTIONS'] || '{}'); } catch (error) { - if (error instanceof Error) { + if (isErrorLike(error)) { console.warn( `Error parsing EXTRA_LAUNCH_OPTIONS: ${error.message}. Skipping.` );