From 1553f19bab6dc3c0b7812d15600f4efbbd3a5db3 Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Tue, 21 Jul 2020 15:25:31 -0700 Subject: [PATCH] chore: update error messages to match future rpc validator (#3075) --- src/browserContext.ts | 18 ++++++++++++------ src/dom.ts | 16 +++++++++------- src/frames.ts | 12 ++++++------ src/page.ts | 4 ++-- src/rpc/client/elementHandle.ts | 4 ++-- src/rpc/client/frame.ts | 8 ++++---- test/emulation.jest.js | 4 ++-- test/geolocation.jest.js | 6 +++--- test/navigation.jest.js | 7 ++++++- test/page.jest.js | 12 ++++++------ test/waittask.jest.js | 6 +++--- 11 files changed, 55 insertions(+), 42 deletions(-) diff --git a/src/browserContext.ts b/src/browserContext.ts index 7a0ffec22f..28da982edd 100644 --- a/src/browserContext.ts +++ b/src/browserContext.ts @@ -290,12 +290,18 @@ export function verifyGeolocation(geolocation: types.Geolocation): types.Geoloca const result = { ...geolocation }; result.accuracy = result.accuracy || 0; const { longitude, latitude, accuracy } = result; - if (!helper.isNumber(longitude) || longitude < -180 || longitude > 180) - throw new Error(`Invalid longitude "${longitude}": precondition -180 <= LONGITUDE <= 180 failed.`); - if (!helper.isNumber(latitude) || latitude < -90 || latitude > 90) - throw new Error(`Invalid latitude "${latitude}": precondition -90 <= LATITUDE <= 90 failed.`); - if (!helper.isNumber(accuracy) || accuracy < 0) - throw new Error(`Invalid accuracy "${accuracy}": precondition 0 <= ACCURACY failed.`); + if (!helper.isNumber(longitude)) + throw new Error(`geolocation.longitude: expected number, got ${typeof longitude}`); + if (longitude < -180 || longitude > 180) + throw new Error(`geolocation.longitude: precondition -180 <= LONGITUDE <= 180 failed.`); + if (!helper.isNumber(latitude)) + throw new Error(`geolocation.latitude: expected number, got ${typeof latitude}`); + if (latitude < -90 || latitude > 90) + throw new Error(`geolocation.latitude: precondition -90 <= LATITUDE <= 90 failed.`); + if (!helper.isNumber(accuracy)) + throw new Error(`geolocation.accuracy: expected number, got ${typeof accuracy}`); + if (accuracy < 0) + throw new Error(`geolocation.accuracy: precondition 0 <= ACCURACY failed.`); return result; } diff --git a/src/dom.ts b/src/dom.ts index 01c9997925..51096ef74c 100644 --- a/src/dom.ts +++ b/src/dom.ts @@ -419,17 +419,19 @@ export class ElementHandle extends js.JSHandle { vals = [ values ] as (string[] | ElementHandle[] | types.SelectOption[]); else vals = values; - const selectOptions = (vals as any).map((value: any) => typeof value === 'object' ? value : { value }); - for (const option of selectOptions) { - assert(option !== null, 'Value items must not be null'); + const selectOptions = (vals as any).map((value: any) => helper.isString(value) ? { value } : value); + for (let i = 0; i < selectOptions.length; i++) { + const option = selectOptions[i]; + assert(option !== null, `options[${i}]: expected object, got null`); + assert(typeof option === 'object', `options[${i}]: expected object, got ${typeof option}`); if (option instanceof ElementHandle) continue; if (option.value !== undefined) - assert(helper.isString(option.value), 'Values must be strings. Found value "' + option.value + '" of type "' + (typeof option.value) + '"'); + assert(helper.isString(option.value), `options[${i}].value: expected string, got ${typeof option.value}`); if (option.label !== undefined) - assert(helper.isString(option.label), 'Labels must be strings. Found label "' + option.label + '" of type "' + (typeof option.label) + '"'); + assert(helper.isString(option.label), `options[${i}].label: expected string, got ${typeof option.label}`); if (option.index !== undefined) - assert(helper.isNumber(option.index), 'Indices must be numbers. Found index "' + option.index + '" of type "' + (typeof option.index) + '"'); + assert(helper.isNumber(option.index), `options[${i}].index: expected number, got ${typeof option.index}`); } return this._page._frameManager.waitForSignalsCreatedBy(progress, options.noWaitAfter, async () => { progress.throwIfAborted(); // Avoid action that has side-effects. @@ -446,7 +448,7 @@ export class ElementHandle extends js.JSHandle { async _fill(progress: Progress, value: string, options: types.NavigatingActionWaitOptions): Promise<'error:notconnected' | 'done'> { progress.logger.info(`elementHandle.fill("${value}")`); - assert(helper.isString(value), 'Value must be string. Found value "' + value + '" of type "' + (typeof value) + '"'); + assert(helper.isString(value), `value: expected string, got ${typeof value}`); return this._page._frameManager.waitForSignalsCreatedBy(progress, options.noWaitAfter, async () => { progress.logger.info(' waiting for element to be visible, enabled and editable'); const poll = await this._evaluateHandleInUtility(([injected, node, value]) => { diff --git a/src/frames.ts b/src/frames.ts index 1cb81b9093..2832cdce5b 100644 --- a/src/frames.ts +++ b/src/frames.ts @@ -412,7 +412,7 @@ export class Frame { async goto(url: string, options: types.GotoOptions = {}): Promise { return runNavigationTask(this, options, this._apiName('goto'), async progress => { - const waitUntil = verifyLifecycle(options.waitUntil === undefined ? 'load' : options.waitUntil); + const waitUntil = verifyLifecycle('waitUntil', options.waitUntil === undefined ? 'load' : options.waitUntil); progress.logger.info(`navigating to "${url}", waiting until "${waitUntil}"`); const headers = (this._page._state.extraHTTPHeaders || {}); let referer = headers['referer'] || headers['Referer']; @@ -457,7 +457,7 @@ export class Frame { async waitForNavigation(options: types.WaitForNavigationOptions = {}): Promise { return runNavigationTask(this, options, this._apiName('waitForNavigation'), async progress => { const toUrl = typeof options.url === 'string' ? ` to "${options.url}"` : ''; - const waitUntil = verifyLifecycle(options.waitUntil === undefined ? 'load' : options.waitUntil); + const waitUntil = verifyLifecycle('waitUntil', options.waitUntil === undefined ? 'load' : options.waitUntil); progress.logger.info(`waiting for navigation${toUrl} until "${waitUntil}"`); const navigationEvent: NavigationEvent = await helper.waitForEvent(progress, this._eventEmitter, kNavigationEvent, (event: NavigationEvent) => { @@ -483,7 +483,7 @@ export class Frame { } async _waitForLoadState(progress: Progress, state: types.LifecycleEvent): Promise { - const waitUntil = verifyLifecycle(state); + const waitUntil = verifyLifecycle('state', state); if (!this._subtreeLifecycleEvents.has(waitUntil)) await helper.waitForEvent(progress, this._eventEmitter, kAddLifecycleEvent, (e: types.LifecycleEvent) => e === waitUntil).promise; } @@ -543,7 +543,7 @@ export class Frame { throw new Error('options.waitFor is not supported, did you mean options.state?'); const { state = 'visible' } = options; if (!['attached', 'detached', 'visible', 'hidden'].includes(state)) - throw new Error(`Unsupported state option "${state}"`); + throw new Error(`state: expected one of (attached|detached|visible|hidden)`); const info = selectors._parseSelector(selector); const task = dom.waitForSelectorTask(info, state); return this._page._runAbortableTask(async progress => { @@ -1126,10 +1126,10 @@ async function runNavigationTask(frame: Frame, options: types.TimeoutOptions, return controller.run(task); } -function verifyLifecycle(waitUntil: types.LifecycleEvent): types.LifecycleEvent { +function verifyLifecycle(name: string, waitUntil: types.LifecycleEvent): types.LifecycleEvent { if (waitUntil as unknown === 'networkidle0') waitUntil = 'networkidle'; if (!types.kLifecycleEvents.has(waitUntil)) - throw new Error(`Unsupported waitUntil option ${String(waitUntil)}`); + throw new Error(`${name}: expected one of (load|domcontentloaded|networkidle)`); return waitUntil; } diff --git a/src/page.ts b/src/page.ts index 803c2a44ee..8442fadfec 100644 --- a/src/page.ts +++ b/src/page.ts @@ -386,8 +386,8 @@ export class Page extends EventEmitter { } async emulateMedia(options: { media?: types.MediaType, colorScheme?: types.ColorScheme }) { - assert(!options.media || types.mediaTypes.has(options.media), 'Unsupported media: ' + options.media); - assert(!options.colorScheme || types.colorSchemes.has(options.colorScheme), 'Unsupported color scheme: ' + options.colorScheme); + assert(!options.media || types.mediaTypes.has(options.media), 'media: expected one of (screen|print)'); + assert(!options.colorScheme || types.colorSchemes.has(options.colorScheme), 'colorScheme: expected one of (dark|light|no-preference)'); if (options.media !== undefined) this._state.mediaType = options.media; if (options.colorScheme !== undefined) diff --git a/src/rpc/client/elementHandle.ts b/src/rpc/client/elementHandle.ts index 9b6c5755fc..f3294d6317 100644 --- a/src/rpc/client/elementHandle.ts +++ b/src/rpc/client/elementHandle.ts @@ -217,8 +217,8 @@ export function convertSelectOptionValues(values: string | ElementHandle | types values = [ values as any ]; if (!values.length) return {}; - if ((values as any[]).includes(null)) - assert(false, 'Value items must not be null'); + for (let i = 0; i < values.length; i++) + assert(values[i] !== null, `options[${i}]: expected object, got null`); if (values[0] instanceof ElementHandle) return { elements: (values as ElementHandle[]).map((v: ElementHandle) => v._elementChannel) }; diff --git a/src/rpc/client/frame.ts b/src/rpc/client/frame.ts index b3f2a4ae8d..1c3443279d 100644 --- a/src/rpc/client/frame.ts +++ b/src/rpc/client/frame.ts @@ -104,7 +104,7 @@ export class Frame extends ChannelOwner { async waitForNavigation(options: types.WaitForNavigationOptions = {}): Promise { return this._wrapApiCall(this._apiName('waitForNavigation'), async () => { - const waitUntil = verifyLoadState(options.waitUntil === undefined ? 'load' : options.waitUntil); + const waitUntil = verifyLoadState('waitUntil', options.waitUntil === undefined ? 'load' : options.waitUntil); const timeout = this._page!._timeoutSettings.navigationTimeout(options); const waiter = this._setupNavigationWaiter(); waiter.rejectOnTimeout(timeout, new TimeoutError(`Timeout ${timeout}ms exceeded.`)); @@ -140,7 +140,7 @@ export class Frame extends ChannelOwner { } async waitForLoadState(state: types.LifecycleEvent = 'load', options: types.TimeoutOptions = {}): Promise { - state = verifyLoadState(state); + state = verifyLoadState('state', state); if (this._loadStates.has(state)) return; return this._wrapApiCall(this._apiName('waitForLoadState'), async () => { @@ -398,10 +398,10 @@ export class Frame extends ChannelOwner { } } -function verifyLoadState(waitUntil: types.LifecycleEvent): types.LifecycleEvent { +function verifyLoadState(name: string, waitUntil: types.LifecycleEvent): types.LifecycleEvent { if (waitUntil as unknown === 'networkidle0') waitUntil = 'networkidle'; if (!types.kLifecycleEvents.has(waitUntil)) - throw new Error(`Unsupported waitUntil option ${String(waitUntil)}`); + throw new Error(`${name}: expected one of (load|domcontentloaded|networkidle)`); return waitUntil; } diff --git a/test/emulation.jest.js b/test/emulation.jest.js index 2ef514e9ee..33c6bd3a28 100644 --- a/test/emulation.jest.js +++ b/test/emulation.jest.js @@ -248,7 +248,7 @@ describe('Page.emulateMedia type', function() { it('should throw in case of bad type argument', async({page, server}) => { let error = null; await page.emulateMedia({ media: 'bad' }).catch(e => error = e); - expect(error.message).toContain('Unsupported media: bad'); + expect(error.message).toContain('media: expected one of (screen|print)'); }); }); @@ -276,7 +276,7 @@ describe('Page.emulateMedia colorScheme', function() { it('should throw in case of bad argument', async({page, server}) => { let error = null; await page.emulateMedia({ colorScheme: 'bad' }).catch(e => error = e); - expect(error.message).toContain('Unsupported color scheme: bad'); + expect(error.message).toContain('colorScheme: expected one of (dark|light|no-preference)'); }); it('should work during navigation', async({page, server}) => { await page.emulateMedia({ colorScheme: 'light' }); diff --git a/test/geolocation.jest.js b/test/geolocation.jest.js index ba7ded4fe4..10700ce538 100644 --- a/test/geolocation.jest.js +++ b/test/geolocation.jest.js @@ -37,7 +37,7 @@ describe('Overrides.setGeolocation', function() { } catch (e) { error = e; } - expect(error.message).toContain('Invalid longitude "200"'); + expect(error.message).toContain('geolocation.longitude: precondition -180 <= LONGITUDE <= 180 failed.'); }); it('should isolate contexts', async({page, server, context, browser}) => { await context.grantPermissions(['geolocation']); @@ -76,7 +76,7 @@ describe('Overrides.setGeolocation', function() { } catch (e) { error = e; } - expect(error.message).toContain('Invalid latitude "undefined"'); + expect(error.message).toContain('geolocation.latitude: expected number, got undefined'); }); it('should not modify passed default options object', async({browser}) => { const geolocation = { longitude: 10, latitude: 10 }; @@ -94,7 +94,7 @@ describe('Overrides.setGeolocation', function() { } catch (e) { error = e; } - expect(error.message).toContain('Invalid longitude "undefined"'); + expect(error.message).toContain('geolocation.longitude: expected number, got undefined'); }); it('should use context options', async({browser, server}) => { const options = { geolocation: { longitude: 10, latitude: 10 }, permissions: ['geolocation'] }; diff --git a/test/navigation.jest.js b/test/navigation.jest.js index 83b93ff30e..28319edfb5 100644 --- a/test/navigation.jest.js +++ b/test/navigation.jest.js @@ -179,7 +179,7 @@ describe('Page.goto', function() { it('should throw if networkidle2 is passed as an option', async({page, server}) => { let error = null; await page.goto(server.EMPTY_PAGE, {waitUntil: 'networkidle2'}).catch(err => error = err); - expect(error.message).toContain('Unsupported waitUntil option'); + expect(error.message).toContain(`waitUntil: expected one of (load|domcontentloaded|networkidle)`); }); it('should fail when main resources failed to load', async({page, server}) => { let error = null; @@ -776,6 +776,11 @@ describe('Page.waitForLoadState', () => { await page.goto(server.PREFIX + '/one-style.html'); await page.waitForLoadState(); }); + it('should throw for bad state', async({page, server}) => { + await page.goto(server.PREFIX + '/one-style.html'); + const error = await page.waitForLoadState('bad').catch(e => e); + expect(error.message).toContain(`state: expected one of (load|domcontentloaded|networkidle)`); + }); it('should resolve immediately if load state matches', async({page, server}) => { await page.goto(server.EMPTY_PAGE); server.setRoute('/one-style.css', (req, res) => response = res); diff --git a/test/page.jest.js b/test/page.jest.js index a94ee48cea..2c27e2525f 100644 --- a/test/page.jest.js +++ b/test/page.jest.js @@ -1003,7 +1003,7 @@ describe('Page.selectOption', function() { await page.evaluate(() => makeMultiple()); let error = null await page.selectOption('select', ['blue', null, 'black','magenta']).catch(e => error = e); - expect(error.message).toContain('Value items must not be null'); + expect(error.message).toContain('options[1]: expected object, got null'); }); it('should unselect with null',async({page, server}) => { await page.goto(server.PREFIX + '/input/select.html'); @@ -1036,7 +1036,7 @@ describe('Page.selectOption', function() { } catch (e) { error = e; } - expect(error.message).toContain('Values must be strings'); + expect(error.message).toContain('options[0]: expected object, got number'); error = null; try { @@ -1044,7 +1044,7 @@ describe('Page.selectOption', function() { } catch (e) { error = e; } - expect(error.message).toContain('Values must be strings'); + expect(error.message).toContain('options[0].value: expected string, got number'); error = null; try { @@ -1052,7 +1052,7 @@ describe('Page.selectOption', function() { } catch (e) { error = e; } - expect(error.message).toContain('Labels must be strings'); + expect(error.message).toContain('options[0].label: expected string, got number'); error = null; try { @@ -1060,7 +1060,7 @@ describe('Page.selectOption', function() { } catch (e) { error = e; } - expect(error.message).toContain('Indices must be numbers'); + expect(error.message).toContain('options[0].index: expected number, got string'); }); // @see https://github.com/GoogleChrome/puppeteer/issues/3327 it('should work when re-defining top-level Event class', async({page, server}) => { @@ -1176,7 +1176,7 @@ describe('Page.fill', function() { let error = null; await page.goto(server.PREFIX + '/input/textarea.html'); await page.fill('textarea', 123).catch(e => error = e); - expect(error.message).toContain('Value must be string.'); + expect(error.message).toContain('value: expected string, got number'); }); it('should retry on disabled element', async({page, server}) => { await page.goto(server.PREFIX + '/input/textarea.html'); diff --git a/test/waittask.jest.js b/test/waittask.jest.js index 54f33b351e..a54ef4d8a6 100644 --- a/test/waittask.jest.js +++ b/test/waittask.jest.js @@ -466,7 +466,7 @@ describe('Frame.waitForSelector', function() { it('should throw for unknown state option', async({page, server}) => { await page.setContent('
test
'); const error = await page.waitForSelector('section', { state: 'foo' }).catch(e => e); - expect(error.message).toContain('Unsupported state option "foo"'); + expect(error.message).toContain('state: expected one of (attached|detached|visible|hidden)'); }); it('should throw for visibility option', async({page, server}) => { await page.setContent('
test
'); @@ -476,12 +476,12 @@ describe('Frame.waitForSelector', function() { it('should throw for true state option', async({page, server}) => { await page.setContent('
test
'); const error = await page.waitForSelector('section', { state: true }).catch(e => e); - expect(error.message).toContain('Unsupported state option "true"'); + expect(error.message).toContain('state: expected one of (attached|detached|visible|hidden)'); }); it('should throw for false state option', async({page, server}) => { await page.setContent('
test
'); const error = await page.waitForSelector('section', { state: false }).catch(e => e); - expect(error.message).toContain('Unsupported state option "false"'); + expect(error.message).toContain('state: expected one of (attached|detached|visible|hidden)'); }); it('should support >> selector syntax', async({page, server}) => { await page.goto(server.EMPTY_PAGE);