This is an archive of the discontinued Mercurial Phabricator instance.

Differential D1860

dispatch: handle IOError when writing to stderr
ClosedPublic

Authored by indygreg on Jan 15 2018, 12:00 AM.

Details

Reviewers
yuja
Group Reviewers
hg-reviewers
Commits
rHG48fe4f56a3b4: dispatch: handle IOError when writing to stderr
Summary

Previously, attempts to write to stderr in dispatch.run() may lead to
an exception being thrown. This would likely be handled by Python's
default exception handler, which would print the exception and exit

Code in this function is already catching IOError for stdout failures
and converting to exit code 255 (-1 & 255 == 255). Why we weren't
doing the same for stderr for the sake of consistency, I don't know.
I do know that chg and hg diverged in behavior here (as the changed
test-basic.t shows).

After this commit, we catch I/O failure on stderr and change the
exit code to 255. chg and hg now behave consistently. As a bonus,
Rust hg also now passes this test.

I'm skeptical at changing the exit code due to failures this late
in the process. I think we should consider preserving the current
exit code - assuming it is non-0. And, we may want to preserve the
exit code completely if the I/O error is EPIPE (and potentially
other special error classes). There's definitely room to tweak
behavior. But for now, let's at least prevent the uncaught exception.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

indygreg created this revision.Jan 15 2018, 12:00 AM
yuja accepted this revision.Jan 15 2018, 8:12 AM
This revision is now accepted and ready to land.Jan 15 2018, 8:12 AM
Closed by commit rHG48fe4f56a3b4: dispatch: handle IOError when writing to stderr (authored by indygreg). · Explain WhyJan 15 2018, 8:25 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mmercurial/dispatch.py (14 lines)
Mtests/test-basic.t (10 lines)

Diff 4831

mercurial/dispatch.py

status = -1 status = -1
if util.safehasattr(req.ui, 'fout'): if util.safehasattr(req.ui, 'fout'):
try: try:
req.ui.fout.flush() req.ui.fout.flush()
except IOError as e: except IOError as e:
err = e err = e
status = -1 status = -1
if util.safehasattr(req.ui, 'ferr'): if util.safehasattr(req.ui, 'ferr'):
try:
if err is not None and err.errno != errno.EPIPE: if err is not None and err.errno != errno.EPIPE:
req.ui.ferr.write('abort: %s\n' % req.ui.ferr.write('abort: %s\n' %
encoding.strtolocal(err.strerror)) encoding.strtolocal(err.strerror))
req.ui.ferr.flush() req.ui.ferr.flush()
# There's not much we can do about an I/O error here. So (possibly)
# change the status code and move on.
except IOError:
status = -1
sys.exit(status & 255) sys.exit(status & 255)
def _initstdio(): def _initstdio():
for fp in (sys.stdin, sys.stdout, sys.stderr): for fp in (sys.stdin, sys.stdout, sys.stderr):
util.setbinary(fp) util.setbinary(fp)
def _getsimilar(symbols, value): def _getsimilar(symbols, value):
sim = lambda x: difflib.SequenceMatcher(None, value, x).ratio() sim = lambda x: difflib.SequenceMatcher(None, value, x).ratio()

tests/test-basic.t

$ hg status 2>/dev/full $ hg status 2>/dev/full
A a A a
$ hg status >/dev/full $ hg status >/dev/full
abort: No space left on device abort: No space left on device
[255] [255]
#endif #endif
#if devfull no-chg #if devfull
$ hg status >/dev/full 2>&1
[1]
$ hg status ENOENT 2>/dev/full
[1]
#endif
#if devfull chg
$ hg status >/dev/full 2>&1 $ hg status >/dev/full 2>&1
[255] [255]
$ hg status ENOENT 2>/dev/full $ hg status ENOENT 2>/dev/full
[255] [255]
#endif #endif
$ hg commit -m test $ hg commit -m test