fix: handle pyinstaller app with no stderr (#1148) #1154
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mxschmitt
approved these changes
Feb 8, 2022
playwright/_impl/_transport.py
Outdated
| return sys.stderr.fileno() | ||
| except (AttributeError, io.UnsupportedOperation): | ||
| # pyinstaller apps may not have stderr |
There was a problem hiding this comment.
Yes, you're right. I was focused only on pyinstaller but it is specific to pythonw.
I will update the comment. Would you prefer this scenario to be handled differently in the method?
There was a problem hiding this comment.
It seems alright, but we could also check above if it's a nullwriter which seems cleaner.
There was a problem hiding this comment.
I've updated the branch as pythonw and pyinstaller scenarios are different.
- In case of
pythonw,sys.stderrisNone - In case of
pyinstaller,sys.stderris an instance ofNullWriterclass from thepyinstallerpackage.
Moved the check for these two situations to early in the try clause. Don't explicitly check for NullWriter class as this is from another package so added a generic check if sys.stderr has attribute closed.
mxschmitt
reviewed
Feb 8, 2022
mxschmitt
reviewed
Feb 8, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Proposed fix for issue #1148.
When playwright is running in an application which was packaged by pyinstaller,
stderrcan be of typeNullWriterwhich has no attributeclosed. Moved access to the attributeclosedinside the followingtryclause to handle theAttributeError.Re-run unit tests after the change an no additional failures seen.