Skip to content

Commit

Permalink
fix: locator abort and timeout errors should include cause
Browse files Browse the repository at this point in the history
And the cause should contain the stacktrace for where the
locator call has started.

Closes #12194
  • Loading branch information
OrKoN committed Apr 4, 2024
1 parent 2b63630 commit e096065
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 11 deletions.
23 changes: 16 additions & 7 deletions packages/puppeteer-core/src/api/locators/locators.ts
Original file line number Diff line number Diff line change
Expand Up @@ -172,19 +172,23 @@ export abstract class Locator<T> extends EventEmitter<LocatorEvents> {
});
},
retryAndRaceWithSignalAndTimer: <T>(
signal?: AbortSignal
signal?: AbortSignal,
cause?: Error
): OperatorFunction<T, T> => {
const candidates = [];
if (signal) {
candidates.push(
fromEvent(signal, 'abort').pipe(
map(() => {
if (signal.reason instanceof Error) {
signal.reason.cause = cause;
}
throw signal.reason;
})
)
);
}
candidates.push(timeout(this._timeout));
candidates.push(timeout(this._timeout, cause));
return pipe(
retry({delay: RETRY_DELAY}),
raceWith<T, never[]>(...candidates)
Expand Down Expand Up @@ -368,6 +372,7 @@ export abstract class Locator<T> extends EventEmitter<LocatorEvents> {
options?: Readonly<LocatorClickOptions>
): Observable<void> {
const signal = options?.signal;
const cause = new Error('Locator.click');
return this._wait(options).pipe(
this.operators.conditions(
[
Expand All @@ -388,7 +393,7 @@ export abstract class Locator<T> extends EventEmitter<LocatorEvents> {
})
);
}),
this.operators.retryAndRaceWithSignalAndTimer(signal)
this.operators.retryAndRaceWithSignalAndTimer(signal, cause)
);
}

Expand All @@ -398,6 +403,7 @@ export abstract class Locator<T> extends EventEmitter<LocatorEvents> {
options?: Readonly<ActionOptions>
): Observable<void> {
const signal = options?.signal;
const cause = new Error('Locator.fill');
return this._wait(options).pipe(
this.operators.conditions(
[
Expand Down Expand Up @@ -521,7 +527,7 @@ export abstract class Locator<T> extends EventEmitter<LocatorEvents> {
})
);
}),
this.operators.retryAndRaceWithSignalAndTimer(signal)
this.operators.retryAndRaceWithSignalAndTimer(signal, cause)
);
}

Expand All @@ -530,6 +536,7 @@ export abstract class Locator<T> extends EventEmitter<LocatorEvents> {
options?: Readonly<ActionOptions>
): Observable<void> {
const signal = options?.signal;
const cause = new Error('Locator.hover');
return this._wait(options).pipe(
this.operators.conditions(
[
Expand All @@ -549,7 +556,7 @@ export abstract class Locator<T> extends EventEmitter<LocatorEvents> {
})
);
}),
this.operators.retryAndRaceWithSignalAndTimer(signal)
this.operators.retryAndRaceWithSignalAndTimer(signal, cause)
);
}

Expand All @@ -558,6 +565,7 @@ export abstract class Locator<T> extends EventEmitter<LocatorEvents> {
options?: Readonly<LocatorScrollOptions>
): Observable<void> {
const signal = options?.signal;
const cause = new Error('Locator.scroll');
return this._wait(options).pipe(
this.operators.conditions(
[
Expand Down Expand Up @@ -590,7 +598,7 @@ export abstract class Locator<T> extends EventEmitter<LocatorEvents> {
})
);
}),
this.operators.retryAndRaceWithSignalAndTimer(signal)
this.operators.retryAndRaceWithSignalAndTimer(signal, cause)
);
}

Expand All @@ -617,9 +625,10 @@ export abstract class Locator<T> extends EventEmitter<LocatorEvents> {
* @public
*/
async waitHandle(options?: Readonly<ActionOptions>): Promise<HandleFor<T>> {
const cause = new Error('Locator.waitHandle');
return await firstValueFrom(
this._wait(options).pipe(
this.operators.retryAndRaceWithSignalAndTimer(options?.signal)
this.operators.retryAndRaceWithSignalAndTimer(options?.signal, cause)
)
);
}
Expand Down
4 changes: 2 additions & 2 deletions packages/puppeteer-core/src/common/Errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ export class PuppeteerError extends Error {
/**
* @internal
*/
constructor(message?: string) {
super(message);
constructor(message?: string, options?: ErrorOptions) {
super(message, options);
this.name = this.constructor.name;
}

Expand Down
4 changes: 2 additions & 2 deletions packages/puppeteer-core/src/common/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -312,12 +312,12 @@ export function validateDialogType(
/**
* @internal
*/
export function timeout(ms: number): Observable<never> {
export function timeout(ms: number, cause?: Error): Observable<never> {
return ms === 0
? NEVER
: timer(ms).pipe(
map(() => {
throw new TimeoutError(`Timed out after waiting ${ms}ms`);
throw new TimeoutError(`Timed out after waiting ${ms}ms`, {cause});
})
);
}
Expand Down

0 comments on commit e096065

Please sign in to comment.