Skip to content

Commit 1553f19

Browse files
dgozmandgozman
authored
chore: update error messages to match future rpc validator (#3075)
1 parent 18cb1c0 commit 1553f19

File tree

11 files changed

+55
-42
lines changed

11 files changed

+55
-42
lines changed

‎src/browserContext.ts

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -290,12 +290,18 @@ export function verifyGeolocation(geolocation: types.Geolocation): types.Geoloca
290290
const result = { ...geolocation };
291291
result.accuracy = result.accuracy || 0;
292292
const { longitude, latitude, accuracy } = result;
293-
if (!helper.isNumber(longitude) || longitude < -180 || longitude > 180)
294-
throw new Error(`Invalid longitude "${longitude}": precondition -180 <= LONGITUDE <= 180 failed.`);
295-
if (!helper.isNumber(latitude) || latitude < -90 || latitude > 90)
296-
throw new Error(`Invalid latitude "${latitude}": precondition -90 <= LATITUDE <= 90 failed.`);
297-
if (!helper.isNumber(accuracy) || accuracy < 0)
298-
throw new Error(`Invalid accuracy "${accuracy}": precondition 0 <= ACCURACY failed.`);
293+
if (!helper.isNumber(longitude))
294+
throw new Error(`geolocation.longitude: expected number, got ${typeof longitude}`);
295+
if (longitude < -180 || longitude > 180)
296+
throw new Error(`geolocation.longitude: precondition -180 <= LONGITUDE <= 180 failed.`);
297+
if (!helper.isNumber(latitude))
298+
throw new Error(`geolocation.latitude: expected number, got ${typeof latitude}`);
299+
if (latitude < -90 || latitude > 90)
300+
throw new Error(`geolocation.latitude: precondition -90 <= LATITUDE <= 90 failed.`);
301+
if (!helper.isNumber(accuracy))
302+
throw new Error(`geolocation.accuracy: expected number, got ${typeof accuracy}`);
303+
if (accuracy < 0)
304+
throw new Error(`geolocation.accuracy: precondition 0 <= ACCURACY failed.`);
299305
return result;
300306
}
301307

‎src/dom.ts

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -419,17 +419,19 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
419419
vals = [ values ] as (string[] | ElementHandle[] | types.SelectOption[]);
420420
else
421421
vals = values;
422-
const selectOptions = (vals as any).map((value: any) => typeof value === 'object' ? value : { value });
423-
for (const option of selectOptions) {
424-
assert(option !== null, 'Value items must not be null');
422+
const selectOptions = (vals as any).map((value: any) => helper.isString(value) ? { value } : value);
423+
for (let i = 0; i < selectOptions.length; i++) {
424+
const option = selectOptions[i];
425+
assert(option !== null, `options[${i}]: expected object, got null`);
426+
assert(typeof option === 'object', `options[${i}]: expected object, got ${typeof option}`);
425427
if (option instanceof ElementHandle)
426428
continue;
427429
if (option.value !== undefined)
428-
assert(helper.isString(option.value), 'Values must be strings. Found value "' + option.value + '" of type "' + (typeof option.value) + '"');
430+
assert(helper.isString(option.value), `options[${i}].value: expected string, got ${typeof option.value}`);
429431
if (option.label !== undefined)
430-
assert(helper.isString(option.label), 'Labels must be strings. Found label "' + option.label + '" of type "' + (typeof option.label) + '"');
432+
assert(helper.isString(option.label), `options[${i}].label: expected string, got ${typeof option.label}`);
431433
if (option.index !== undefined)
432-
assert(helper.isNumber(option.index), 'Indices must be numbers. Found index "' + option.index + '" of type "' + (typeof option.index) + '"');
434+
assert(helper.isNumber(option.index), `options[${i}].index: expected number, got ${typeof option.index}`);
433435
}
434436
return this._page._frameManager.waitForSignalsCreatedBy(progress, options.noWaitAfter, async () => {
435437
progress.throwIfAborted(); // Avoid action that has side-effects.
@@ -446,7 +448,7 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
446448

447449
async _fill(progress: Progress, value: string, options: types.NavigatingActionWaitOptions): Promise<'error:notconnected' | 'done'> {
448450
progress.logger.info(`elementHandle.fill("${value}")`);
449-
assert(helper.isString(value), 'Value must be string. Found value "' + value + '" of type "' + (typeof value) + '"');
451+
assert(helper.isString(value), `value: expected string, got ${typeof value}`);
450452
return this._page._frameManager.waitForSignalsCreatedBy(progress, options.noWaitAfter, async () => {
451453
progress.logger.info(' waiting for element to be visible, enabled and editable');
452454
const poll = await this._evaluateHandleInUtility(([injected, node, value]) => {

‎src/frames.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -412,7 +412,7 @@ export class Frame {
412412

413413
async goto(url: string, options: types.GotoOptions = {}): Promise<network.Response | null> {
414414
return runNavigationTask(this, options, this._apiName('goto'), async progress => {
415-
const waitUntil = verifyLifecycle(options.waitUntil === undefined ? 'load' : options.waitUntil);
415+
const waitUntil = verifyLifecycle('waitUntil', options.waitUntil === undefined ? 'load' : options.waitUntil);
416416
progress.logger.info(`navigating to "${url}", waiting until "${waitUntil}"`);
417417
const headers = (this._page._state.extraHTTPHeaders || {});
418418
let referer = headers['referer'] || headers['Referer'];
@@ -457,7 +457,7 @@ export class Frame {
457457
async waitForNavigation(options: types.WaitForNavigationOptions = {}): Promise<network.Response | null> {
458458
return runNavigationTask(this, options, this._apiName('waitForNavigation'), async progress => {
459459
const toUrl = typeof options.url === 'string' ? ` to "${options.url}"` : '';
460-
const waitUntil = verifyLifecycle(options.waitUntil === undefined ? 'load' : options.waitUntil);
460+
const waitUntil = verifyLifecycle('waitUntil', options.waitUntil === undefined ? 'load' : options.waitUntil);
461461
progress.logger.info(`waiting for navigation${toUrl} until "${waitUntil}"`);
462462

463463
const navigationEvent: NavigationEvent = await helper.waitForEvent(progress, this._eventEmitter, kNavigationEvent, (event: NavigationEvent) => {
@@ -483,7 +483,7 @@ export class Frame {
483483
}
484484

485485
async _waitForLoadState(progress: Progress, state: types.LifecycleEvent): Promise<void> {
486-
const waitUntil = verifyLifecycle(state);
486+
const waitUntil = verifyLifecycle('state', state);
487487
if (!this._subtreeLifecycleEvents.has(waitUntil))
488488
await helper.waitForEvent(progress, this._eventEmitter, kAddLifecycleEvent, (e: types.LifecycleEvent) => e === waitUntil).promise;
489489
}
@@ -543,7 +543,7 @@ export class Frame {
543543
throw new Error('options.waitFor is not supported, did you mean options.state?');
544544
const { state = 'visible' } = options;
545545
if (!['attached', 'detached', 'visible', 'hidden'].includes(state))
546-
throw new Error(`Unsupported state option "${state}"`);
546+
throw new Error(`state: expected one of (attached|detached|visible|hidden)`);
547547
const info = selectors._parseSelector(selector);
548548
const task = dom.waitForSelectorTask(info, state);
549549
return this._page._runAbortableTask(async progress => {
@@ -1126,10 +1126,10 @@ async function runNavigationTask<T>(frame: Frame, options: types.TimeoutOptions,
11261126
return controller.run(task);
11271127
}
11281128

1129-
function verifyLifecycle(waitUntil: types.LifecycleEvent): types.LifecycleEvent {
1129+
function verifyLifecycle(name: string, waitUntil: types.LifecycleEvent): types.LifecycleEvent {
11301130
if (waitUntil as unknown === 'networkidle0')
11311131
waitUntil = 'networkidle';
11321132
if (!types.kLifecycleEvents.has(waitUntil))
1133-
throw new Error(`Unsupported waitUntil option ${String(waitUntil)}`);
1133+
throw new Error(`${name}: expected one of (load|domcontentloaded|networkidle)`);
11341134
return waitUntil;
11351135
}

‎src/page.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -386,8 +386,8 @@ export class Page extends EventEmitter {
386386
}
387387

388388
async emulateMedia(options: { media?: types.MediaType, colorScheme?: types.ColorScheme }) {
389-
assert(!options.media || types.mediaTypes.has(options.media), 'Unsupported media: ' + options.media);
390-
assert(!options.colorScheme || types.colorSchemes.has(options.colorScheme), 'Unsupported color scheme: ' + options.colorScheme);
389+
assert(!options.media || types.mediaTypes.has(options.media), 'media: expected one of (screen|print)');
390+
assert(!options.colorScheme || types.colorSchemes.has(options.colorScheme), 'colorScheme: expected one of (dark|light|no-preference)');
391391
if (options.media !== undefined)
392392
this._state.mediaType = options.media;
393393
if (options.colorScheme !== undefined)

‎src/rpc/client/elementHandle.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -217,8 +217,8 @@ export function convertSelectOptionValues(values: string | ElementHandle | types
217217
values = [ values as any ];
218218
if (!values.length)
219219
return {};
220-
if ((values as any[]).includes(null))
221-
assert(false, 'Value items must not be null');
220+
for (let i = 0; i < values.length; i++)
221+
assert(values[i] !== null, `options[${i}]: expected object, got null`);
222222

223223
if (values[0] instanceof ElementHandle)
224224
return { elements: (values as ElementHandle[]).map((v: ElementHandle) => v._elementChannel) };

‎src/rpc/client/frame.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ export class Frame extends ChannelOwner<FrameChannel, FrameInitializer> {
104104

105105
async waitForNavigation(options: types.WaitForNavigationOptions = {}): Promise<network.Response | null> {
106106
return this._wrapApiCall(this._apiName('waitForNavigation'), async () => {
107-
const waitUntil = verifyLoadState(options.waitUntil === undefined ? 'load' : options.waitUntil);
107+
const waitUntil = verifyLoadState('waitUntil', options.waitUntil === undefined ? 'load' : options.waitUntil);
108108
const timeout = this._page!._timeoutSettings.navigationTimeout(options);
109109
const waiter = this._setupNavigationWaiter();
110110
waiter.rejectOnTimeout(timeout, new TimeoutError(`Timeout ${timeout}ms exceeded.`));
@@ -140,7 +140,7 @@ export class Frame extends ChannelOwner<FrameChannel, FrameInitializer> {
140140
}
141141

142142
async waitForLoadState(state: types.LifecycleEvent = 'load', options: types.TimeoutOptions = {}): Promise<void> {
143-
state = verifyLoadState(state);
143+
state = verifyLoadState('state', state);
144144
if (this._loadStates.has(state))
145145
return;
146146
return this._wrapApiCall(this._apiName('waitForLoadState'), async () => {
@@ -398,10 +398,10 @@ export class Frame extends ChannelOwner<FrameChannel, FrameInitializer> {
398398
}
399399
}
400400

401-
function verifyLoadState(waitUntil: types.LifecycleEvent): types.LifecycleEvent {
401+
function verifyLoadState(name: string, waitUntil: types.LifecycleEvent): types.LifecycleEvent {
402402
if (waitUntil as unknown === 'networkidle0')
403403
waitUntil = 'networkidle';
404404
if (!types.kLifecycleEvents.has(waitUntil))
405-
throw new Error(`Unsupported waitUntil option ${String(waitUntil)}`);
405+
throw new Error(`${name}: expected one of (load|domcontentloaded|networkidle)`);
406406
return waitUntil;
407407
}

‎test/emulation.jest.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -248,7 +248,7 @@ describe('Page.emulateMedia type', function() {
248248
it('should throw in case of bad type argument', async({page, server}) => {
249249
let error = null;
250250
await page.emulateMedia({ media: 'bad' }).catch(e => error = e);
251-
expect(error.message).toContain('Unsupported media: bad');
251+
expect(error.message).toContain('media: expected one of (screen|print)');
252252
});
253253
});
254254

@@ -276,7 +276,7 @@ describe('Page.emulateMedia colorScheme', function() {
276276
it('should throw in case of bad argument', async({page, server}) => {
277277
let error = null;
278278
await page.emulateMedia({ colorScheme: 'bad' }).catch(e => error = e);
279-
expect(error.message).toContain('Unsupported color scheme: bad');
279+
expect(error.message).toContain('colorScheme: expected one of (dark|light|no-preference)');
280280
});
281281
it('should work during navigation', async({page, server}) => {
282282
await page.emulateMedia({ colorScheme: 'light' });

‎test/geolocation.jest.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ describe('Overrides.setGeolocation', function() {
3737
} catch (e) {
3838
error = e;
3939
}
40-
expect(error.message).toContain('Invalid longitude "200"');
40+
expect(error.message).toContain('geolocation.longitude: precondition -180 <= LONGITUDE <= 180 failed.');
4141
});
4242
it('should isolate contexts', async({page, server, context, browser}) => {
4343
await context.grantPermissions(['geolocation']);
@@ -76,7 +76,7 @@ describe('Overrides.setGeolocation', function() {
7676
} catch (e) {
7777
error = e;
7878
}
79-
expect(error.message).toContain('Invalid latitude "undefined"');
79+
expect(error.message).toContain('geolocation.latitude: expected number, got undefined');
8080
});
8181
it('should not modify passed default options object', async({browser}) => {
8282
const geolocation = { longitude: 10, latitude: 10 };
@@ -94,7 +94,7 @@ describe('Overrides.setGeolocation', function() {
9494
} catch (e) {
9595
error = e;
9696
}
97-
expect(error.message).toContain('Invalid longitude "undefined"');
97+
expect(error.message).toContain('geolocation.longitude: expected number, got undefined');
9898
});
9999
it('should use context options', async({browser, server}) => {
100100
const options = { geolocation: { longitude: 10, latitude: 10 }, permissions: ['geolocation'] };

‎test/navigation.jest.js

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -179,7 +179,7 @@ describe('Page.goto', function() {
179179
it('should throw if networkidle2 is passed as an option', async({page, server}) => {
180180
let error = null;
181181
await page.goto(server.EMPTY_PAGE, {waitUntil: 'networkidle2'}).catch(err => error = err);
182-
expect(error.message).toContain('Unsupported waitUntil option');
182+
expect(error.message).toContain(`waitUntil: expected one of (load|domcontentloaded|networkidle)`);
183183
});
184184
it('should fail when main resources failed to load', async({page, server}) => {
185185
let error = null;
@@ -776,6 +776,11 @@ describe('Page.waitForLoadState', () => {
776776
await page.goto(server.PREFIX + '/one-style.html');
777777
await page.waitForLoadState();
778778
});
779+
it('should throw for bad state', async({page, server}) => {
780+
await page.goto(server.PREFIX + '/one-style.html');
781+
const error = await page.waitForLoadState('bad').catch(e => e);
782+
expect(error.message).toContain(`state: expected one of (load|domcontentloaded|networkidle)`);
783+
});
779784
it('should resolve immediately if load state matches', async({page, server}) => {
780785
await page.goto(server.EMPTY_PAGE);
781786
server.setRoute('/one-style.css', (req, res) => response = res);

‎test/page.jest.js

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1003,7 +1003,7 @@ describe('Page.selectOption', function() {
10031003
await page.evaluate(() => makeMultiple());
10041004
let error = null
10051005
await page.selectOption('select', ['blue', null, 'black','magenta']).catch(e => error = e);
1006-
expect(error.message).toContain('Value items must not be null');
1006+
expect(error.message).toContain('options[1]: expected object, got null');
10071007
});
10081008
it('should unselect with null',async({page, server}) => {
10091009
await page.goto(server.PREFIX + '/input/select.html');
@@ -1036,31 +1036,31 @@ describe('Page.selectOption', function() {
10361036
} catch (e) {
10371037
error = e;
10381038
}
1039-
expect(error.message).toContain('Values must be strings');
1039+
expect(error.message).toContain('options[0]: expected object, got number');
10401040

10411041
error = null;
10421042
try {
10431043
await page.selectOption('select', { value: 12 });
10441044
} catch (e) {
10451045
error = e;
10461046
}
1047-
expect(error.message).toContain('Values must be strings');
1047+
expect(error.message).toContain('options[0].value: expected string, got number');
10481048

10491049
error = null;
10501050
try {
10511051
await page.selectOption('select', { label: 12 });
10521052
} catch (e) {
10531053
error = e;
10541054
}
1055-
expect(error.message).toContain('Labels must be strings');
1055+
expect(error.message).toContain('options[0].label: expected string, got number');
10561056

10571057
error = null;
10581058
try {
10591059
await page.selectOption('select', { index: '12' });
10601060
} catch (e) {
10611061
error = e;
10621062
}
1063-
expect(error.message).toContain('Indices must be numbers');
1063+
expect(error.message).toContain('options[0].index: expected number, got string');
10641064
});
10651065
// @see https://github.com/GoogleChrome/puppeteer/issues/3327
10661066
it('should work when re-defining top-level Event class', async({page, server}) => {
@@ -1176,7 +1176,7 @@ describe('Page.fill', function() {
11761176
let error = null;
11771177
await page.goto(server.PREFIX + '/input/textarea.html');
11781178
await page.fill('textarea', 123).catch(e => error = e);
1179-
expect(error.message).toContain('Value must be string.');
1179+
expect(error.message).toContain('value: expected string, got number');
11801180
});
11811181
it('should retry on disabled element', async({page, server}) => {
11821182
await page.goto(server.PREFIX + '/input/textarea.html');

0 commit comments

Comments
 (0)