Skip to content

Conversation

@xg-wang
Copy link
Contributor

@xg-wang xg-wang commented Nov 6, 2017

fix #1221

@xg-wang
Copy link
Contributor Author

xg-wang commented Nov 6, 2017

Reject onerror as discussed in the issue.
Add by path has already been taken care by the readFileAsync, only add by url is considered here.

Copy link
Contributor

Choose a reason for hiding this comment

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

a few nits:

  1. there should be no need to extract a "handle" variable
  2. I'd extract the "url" into a separate variable since options.url might've changed by client code at the moment of throwing error.
const url = options.url;
try {
  return await this._context.evaluateHandle(addScriptUrl, url);
} catch (error) {
  throw new Error(`Loading script from ${url} failed`);
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks! This makes sense.

Copy link
Contributor

@aslushnikov aslushnikov left a comment

Choose a reason for hiding this comment

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

Thanks!

@aslushnikov aslushnikov merged commit 8e44573 into puppeteer:master Nov 7, 2017
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.

calling addScriptTag returns Promise that is not rejected in case of an error

2 participants