Skip to content

misc: preserve error stack when using promise timeout #13333

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
adamraine merged 4 commits into from
Nov 9, 2021
Merged

misc: preserve error stack when using promise timeout #13333

adamraine merged 4 commits into from
Nov 9, 2021

Conversation

@BR0kEN-
Copy link
Contributor

@BR0kEN- BR0kEN- commented Nov 6, 2021

@BR0kEN- BR0kEN- requested a review from a team as a code owner November 6, 2021 18:27
@BR0kEN- BR0kEN- requested review from adamraine and removed request for a team November 6, 2021 18:27
@google-cla google-cla bot added the cla: yes label Nov 6, 2021
@BR0kEN- BR0kEN- changed the title Resolves #13332: Preserve error stack when timing out a promise #13332: Preserve error stack when timing out a promise Nov 6, 2021
@BR0kEN- BR0kEN- changed the title #13332: Preserve error stack when timing out a promise promise.race: Preserve error stack when timing out a promise Nov 6, 2021
@BR0kEN- BR0kEN- changed the title promise.race: Preserve error stack when timing out a promise promise.race: preserve error stack when timing out a promise Nov 6, 2021
@adamraine
Copy link
Contributor

adamraine commented Nov 8, 2021

This looks good! While you're here, I think there is one more usage of error timeout that we can upgrade:

lighthouse/lighthouse-cli/run.js

Lines 175 to 179 in 68ba77a

/** @type {NodeJS.Timeout} */
let timeout;
const timeoutPromise = new Promise((_, reject) => {
timeout = setTimeout(() => reject(new Error('Timed out waiting to kill Chrome')), 5000);
});

@adamraine adamraine changed the title promise.race: preserve error stack when timing out a promise misc: preserve error stack when timing out a promise Nov 8, 2021
@BR0kEN-
Copy link
Contributor Author

BR0kEN- commented Nov 8, 2021

Yeah, true, I was searching across lighthouse-core only. Give me a couple of mins.

@connorjclark connorjclark changed the title misc: preserve error stack when timing out a promise misc: preserve error stack when using promise timeout Nov 8, 2021
@BR0kEN-
Copy link
Contributor Author

BR0kEN- commented Nov 9, 2021

All failed checks seem to be unrelated to changes. What are the next actions @adamraine?

@adamraine
Copy link
Contributor

adamraine commented Nov 9, 2021

Correct, failed checks are unrelated. #13342 should fix the smoke failure, after that we can land this.

@adamraine adamraine merged commit 665df67 into GoogleChrome:master Nov 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@adamraine adamraine adamraine approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@adamraine adamraine

Labels

cla: yes waiting4reviewer

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@BR0kEN- @adamraine @devtools-bot @connorjclark