Skip to content
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.

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

fix(page): dispatch errors into page #3550

Merged
merged 4 commits into from Nov 15, 2018

Conversation

aslushnikov
Copy link
Contributor

Errors thrown on the node side of the page.exposeFunction callback
should be dispatched into the page.

Fixes #3549

Errors thrown on the node side of the `page.exposeFunction` callback
should be dispatched into the page.

Fixes puppeteer#3549
@aslushnikov aslushnikov changed the title fix(page.exposeFunction): dispatch errors into page fix(page): dispatch errors into page Nov 15, 2018
lib/Page.js Outdated
}

function deliverError(name, seq, message, stack) {
let error = new Error(message);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Const

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

lib/Page.js Outdated
error = e;
}
let expression = null;
if (error)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Things can throw null. Instead of checking if there is an error, do the logic in the catch.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair. Creepy, but fair.

window[name]['callbacks'].delete(seq);
}

function deliverError(name, seq, message, stack) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: JSDoc

@JoelEinbinder
Copy link
Collaborator

oh, we should probably pass the name as well. For TimeoutErrors

@aslushnikov aslushnikov merged commit 95a19c7 into puppeteer:master Nov 15, 2018
@@ -428,7 +428,7 @@ class Page extends EventEmitter {
}
const seq = (me['lastSeq'] || 0) + 1;
me['lastSeq'] = seq;
const promise = new Promise(fulfill => callbacks.set(seq, fulfill));

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok

@aslushnikov aslushnikov deleted the throw-proper-errors branch January 22, 2019 20:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants