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(firefox): fix launching firefox without dependencies #2900

Conversation

aslushnikov
Copy link
Collaborator

We always have to reject promises with some error. Otherwise,
our error-rewriting logic in try-catch miserably fails.

With this patch, attempt to launch Firefox when it's missing
dependencies will actually result in a thrown exception with
pretty logs. Without this patch, Playwright throws internal error.

Copy link
Member

@yury-s yury-s left a comment

Choose a reason for hiding this comment

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

can we test this by running a "binary" that fails?

@aslushnikov
Copy link
Collaborator Author

Yep, trying!

helper.addEventListener(rl, 'close', reject),
helper.addEventListener(process, 'exit', reject),
helper.addEventListener(process, 'error', reject)
helper.addEventListener(rl, 'close', reject.bind(null, failError)),
Copy link
Member

Choose a reason for hiding this comment

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

nit: extract in a variable?

We always have to reject promises with some error. Otherwise,
our error-rewriting logic in try-catch miserably fails.

With this patch, attempt to launch Firefox when it's missing
dependencies will actually result in a thrown exception with
pretty logs. Without this patch, Playwright throws internal error.
@aslushnikov aslushnikov force-pushed the fix-launching-firefox-without-dependencies branch from 3399b69 to 8473a14 Compare July 10, 2020 00:58
@aslushnikov aslushnikov merged commit a403d4b into microsoft:master Jul 10, 2020
@aslushnikov aslushnikov deleted the fix-launching-firefox-without-dependencies branch July 10, 2020 23:04
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

2 participants