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.

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

Fix errors on Close and Wait when browser closes #419

Merged
merged 1 commit into from
Jun 28, 2022
Merged

Fix errors on Close and Wait when browser closes #419

merged 1 commit into from
Jun 28, 2022

Conversation

ankur22
Copy link
Collaborator

@ankur22 ankur22 commented Jun 27, 2022

@ankur22 ankur22 requested review from inancgumus and imiric June 27, 2022 19:12
@ankur22 ankur22 added the bug Something isn't working label Jun 27, 2022
@ankur22 ankur22 self-assigned this Jun 27, 2022
@ankur22 ankur22 added this to the v0.4.0 milestone Jun 27, 2022
imiric
imiric reviewed Jun 28, 2022
View reviewed changes
chromium/browser_type.go Outdated Show resolved Hide resolved
@ankur22 ankur22 requested a review from imiric June 28, 2022 11:53
ankur22 added a commit that referenced this pull request Jun 28, 2022
@ankur22
Reformat logging messages
414d93b 
Resolves: #419 (comment)
ankur22 added a commit that referenced this pull request Jun 28, 2022
@ankur22
Reformat logging messages
5222abb 
Resolves: #419 (comment)
@ankur22 ankur22 force-pushed the bug/415_close_error branch from 414d93b to 5222abb Compare June 28, 2022 12:09
imiric
imiric reviewed Jun 28, 2022
View reviewed changes
Copy link
Contributor

@imiric imiric left a comment

Choose a reason for hiding this comment

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

LGTM, assuming it works on Windows. 🤞

Just left minor suggestions.

chromium/browser_type.go Outdated Show resolved Hide resolved
chromium/browser_type.go Outdated Show resolved Hide resolved
@ankur22 ankur22 mentioned this pull request Jun 28, 2022
Closed
@ankur22 ankur22 requested a review from imiric June 28, 2022 15:59
ankur22 added a commit that referenced this pull request Jun 28, 2022
@ankur22
Reformat based on PR suggestions
7832ec8 
Resolves: #419 (comment)
Resolves: #419 (comment)
imiric
imiric reviewed Jun 28, 2022
View reviewed changes
chromium/browser_type.go Outdated Show resolved Hide resolved
ankur22 added a commit that referenced this pull request Jun 28, 2022
@ankur22
Add link to issue for graceful browser shutdown
b87e971 
Resolves: #419 (comment)
imiric
imiric approved these changes Jun 28, 2022
View reviewed changes
Copy link
Contributor

@imiric imiric left a comment

Choose a reason for hiding this comment

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

LGTM 👍

BTW, this is a good candidate for squash+merge.

@ankur22
Fix errors on Close and Wait when browser closes
5750799 
- Close on the stdout pipe is already dealt with by Wait.
- Kill signal is sent when the context is closed so only log other
non kill related errors.

Fixes: #415
Resolves: #419 (comment)
Resolves: #419 (comment)
Resolves: #419 (comment)
Resolves: #419 (comment)
@ankur22 ankur22 force-pushed the bug/415_close_error branch from b87e971 to 5750799 Compare June 28, 2022 16:28
@ankur22 ankur22 merged commit 3c2a566 into main Jun 28, 2022
@ankur22 ankur22 deleted the bug/415_close_error branch June 28, 2022 17:45
ankur22 added a commit that referenced this pull request Jun 28, 2022
@ankur22
Add PR #419 to the Internals release notes
4c01fc6
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@imiric imiric imiric approved these changes

@inancgumus inancgumus inancgumus approved these changes

Assignees

@ankur22 ankur22

Labels
bug Something isn't working
Projects
None yet
Milestone
v0.4.0
Development

Successfully merging this pull request may close these issues.

Browser process close errors when running more than 1 iteration
3 participants
@ankur22 @inancgumus @imiric