[mozprocess] TypeError when using ProcessHandler without the `processOutputLine` argument on Python 3
Categories
(Testing :: Mozbase, defect, P3)
Tracking
(firefox79 fixed)
Tracking | Status | |
---|---|---|
firefox79 | --- | fixed |
People
(Reporter: ahal, Assigned: hamzah18051)
References
Details
Attachments
(1 file)
STR:
In [1]: from mozprocess import ProcessHandler
In [2]: proc = ProcessHandler(["echo", "foo"])
In [3]: proc.run()
Exception in thread ProcessReader:
Traceback (most recent call last):
File "/home/ahal/.pyenv/versions/3.8.2/lib/python3.8/threading.py", line 932, in _bootstrap_inner
self.run()
File "/home/ahal/.pyenv/versions/3.8.2/lib/python3.8/threading.py", line 870, in run
self._target(*self._args, **self._kwargs)
File "/home/ahal/dev/mozilla-central/testing/mozbase/mozprocess/mozprocess/processhandler.py", line 1099, in _read
callback(line.rstrip())
File "/home/ahal/dev/mozilla-central/testing/mozbase/mozprocess/mozprocess/processhandler.py", line 1009, in __call__
e(*args, **kwargs)
File "/home/ahal/dev/mozilla-central/testing/mozbase/mozprocess/mozprocess/processhandler.py", line 1150, in __call__
self.stream.write(line + '\n'.encode('utf8'))
TypeError: write() argument must be str, not bytes
This is happening because we set up a sys.stdout
stream by default here:
https://searchfox.org/mozilla-central/source/testing/mozbase/mozprocess/mozprocess/processhandler.py#1205
But on Python 3, sys.stdout
requires text not bytes (whereas, the default subprocess output remains bytes).
In general, I think it's reasonable to fail if a user requests bytes from the subprocess and then tries to set up a stream that needs text (or vice versa). But we at least need to fix our default case there.
I think we need something like:
if PY3 and not (kwargs.get("universal_newlines") or kwargs.get("text")):
use sys.stdout.buffer as the stream (unsure if this has other negative consequences)
else:
use sys.stdout as the stream
Reporter | ||
Comment 1•5 years ago
|
||
Alternatively we could try/except
the call to stream.write()
and decode the line if necessary. This way users wouldn't need to worry about string types at all.. Though part of me feels that this shouldn't be mozprocess' responsibility.
Comment 2•4 years ago
|
||
Because this bug's Severity has not been changed from the default since it was filed, and it's Priority is P3
(Backlog,) indicating it has been triaged, the bug's Severity is being updated to S3
(normal.)
Assignee | ||
Comment 3•4 years ago
|
||
Updated•4 years ago
|
Updated•4 years ago
|
Comment 5•4 years ago
|
||
bugherder |
Description
•