Patchwork [2,of,3] py3: implement __bytes__() on most of our exception classes

login
register
mail settings
Submitter Yuya Nishihara
Date June 1, 2017, 2:35 p.m.
Message ID <4bdbaa7eb382e3bff59f.1496327746@mimosa>
Download mbox | patch
Permalink /patch/21126/
State Accepted
Headers show

Comments

Yuya Nishihara - June 1, 2017, 2:35 p.m.
# HG changeset patch
# User Yuya Nishihara <yuya@tcha.org>
# Date 1496324604 -32400
#      Thu Jun 01 22:43:24 2017 +0900
# Node ID 4bdbaa7eb382e3bff59f05342ac6a5a5fcac42ed
# Parent  80919de85aade5f931d6fd5c8231f490c0b81125
py3: implement __bytes__() on most of our exception classes

We store bytes in exc.args, which should be translated to a byte string
without encode/decode dance.

IOError subclasses are unchanged for now. We'll need to decide how our
IOErrors should be caught.
Pulkit Goyal - June 2, 2017, 1:56 a.m.
The patch and the idea is awesome, thanks!

On Thu, Jun 1, 2017 at 8:05 PM, Yuya Nishihara <yuya@tcha.org> wrote:
> # HG changeset patch
> # User Yuya Nishihara <yuya@tcha.org>
> # Date 1496324604 -32400
> #      Thu Jun 01 22:43:24 2017 +0900
> # Node ID 4bdbaa7eb382e3bff59f05342ac6a5a5fcac42ed
> # Parent  80919de85aade5f931d6fd5c8231f490c0b81125
> py3: implement __bytes__() on most of our exception classes
>
> We store bytes in exc.args, which should be translated to a byte string
> without encode/decode dance.
>
> IOError subclasses are unchanged for now. We'll need to decide how our
> IOErrors should be caught.
>
> diff --git a/mercurial/error.py b/mercurial/error.py
> --- a/mercurial/error.py
> +++ b/mercurial/error.py
> @@ -13,7 +13,16 @@ imports.
>
>  from __future__ import absolute_import
>
> -# Do not import anything here, please
> +# Do not import anything but pycompat here, please
> +from . import pycompat
> +
> +def _tobytes(exc):
> +    """Byte-stringify exception in the same way as BaseException_str()"""
> +    if not exc.args:
> +        return b''
> +    if len(exc.args) == 1:
> +        return pycompat.bytestr(exc.args[0])
> +    return b'(%s)' % b', '.join(b"'%s'" % pycompat.bytestr(a) for a in exc.args)
>
>  class Hint(object):
>      """Mix-in to provide a hint of an error
> @@ -26,10 +35,10 @@ class Hint(object):
>          super(Hint, self).__init__(*args, **kw)
>
>  class RevlogError(Hint, Exception):
> -    pass
> +    __bytes__ = _tobytes
>
>  class FilteredIndexError(IndexError):
> -    pass
> +    __bytes__ = _tobytes
>
>  class LookupError(RevlogError, KeyError):
>      def __init__(self, name, index, message):
> @@ -43,6 +52,9 @@ class LookupError(RevlogError, KeyError)
>              name = short(name)
>          RevlogError.__init__(self, '%s@%s: %s' % (index, name, message))
>
> +    def __bytes__(self):
> +        return RevlogError.__bytes__(self)
> +
>      def __str__(self):
>          return RevlogError.__str__(self)
>
> @@ -54,12 +66,15 @@ class ManifestLookupError(LookupError):
>
>  class CommandError(Exception):
>      """Exception raised on errors in parsing the command line."""
> +    __bytes__ = _tobytes
>
>  class InterventionRequired(Hint, Exception):
>      """Exception raised when a command requires human intervention."""
> +    __bytes__ = _tobytes
>
>  class Abort(Hint, Exception):
>      """Raised if a command needs to print an error and exit."""
> +    __bytes__ = _tobytes
>
>  class HookLoadError(Abort):
>      """raised when loading a hook fails, aborting an operation
> @@ -94,9 +109,11 @@ class ResponseExpected(Abort):
>
>  class OutOfBandError(Hint, Exception):
>      """Exception raised when a remote repo reports failure"""
> +    __bytes__ = _tobytes
>
>  class ParseError(Hint, Exception):
>      """Raised when parsing config files and {rev,file}sets (msg[, pos])"""
> +    __bytes__ = _tobytes
>
>  class UnknownIdentifier(ParseError):
>      """Exception raised when a {rev,file}set references an unknown identifier"""
> @@ -108,7 +125,7 @@ class UnknownIdentifier(ParseError):
>          self.symbols = symbols
>
>  class RepoError(Hint, Exception):
> -    pass
> +    __bytes__ = _tobytes
>
>  class RepoLookupError(RepoError):
>      pass
> @@ -128,6 +145,8 @@ class StdioError(IOError):
>      def __init__(self, err):
>          IOError.__init__(self, err.errno, err.strerror)
>
> +    # no __bytes__() because error message is derived from the standard IOError
> +
>  class UnsupportedMergeRecords(Abort):
>      def __init__(self, recordtypes):
>          from .i18n import _
> @@ -151,6 +170,8 @@ class LockError(IOError):
>          IOError.__init__(self, errno, strerror, filename)
>          self.desc = desc
>
> +    # no __bytes__() because error message is derived from the standard IOError
> +
>  class LockHeld(LockError):
>      def __init__(self, errno, filename, desc, locker):
>          LockError.__init__(self, errno, 'Lock held', filename, desc)
> @@ -161,36 +182,43 @@ class LockUnavailable(LockError):
>
>  # LockError is for errors while acquiring the lock -- this is unrelated
>  class LockInheritanceContractViolation(RuntimeError):
> -    pass
> +    __bytes__ = _tobytes
>
>  class ResponseError(Exception):
>      """Raised to print an error with part of output and exit."""
> +    __bytes__ = _tobytes
>
>  class UnknownCommand(Exception):
>      """Exception raised if command is not in the command table."""
> +    __bytes__ = _tobytes
>
>  class AmbiguousCommand(Exception):
>      """Exception raised if command shortcut matches more than one command."""
> +    __bytes__ = _tobytes
>
>  # derived from KeyboardInterrupt to simplify some breakout code
>  class SignalInterrupt(KeyboardInterrupt):
>      """Exception raised on SIGTERM and SIGHUP."""
>
>  class SignatureError(Exception):
> -    pass
> +    __bytes__ = _tobytes
>
>  class PushRaced(RuntimeError):
>      """An exception raised during unbundling that indicate a push race"""
> +    __bytes__ = _tobytes
>
>  class ProgrammingError(Hint, RuntimeError):
>      """Raised if a mercurial (core or extension) developer made a mistake"""
> +    __bytes__ = _tobytes
>
>  class WdirUnsupported(Exception):
>      """An exception which is raised when 'wdir()' is not supported"""
> +    __bytes__ = _tobytes
>
>  # bundle2 related errors
>  class BundleValueError(ValueError):
>      """error raised when bundle2 cannot be processed"""
> +    __bytes__ = _tobytes
>
>  class BundleUnknownFeatureError(BundleValueError):
>      def __init__(self, parttype=None, params=(), values=()):
> @@ -217,6 +245,7 @@ class BundleUnknownFeatureError(BundleVa
>
>  class ReadOnlyPartError(RuntimeError):
>      """error raised when code tries to alter a part being generated"""
> +    __bytes__ = _tobytes
>
>  class PushkeyFailed(Abort):
>      """error raised when a pushkey part failed to update a value"""
> @@ -257,12 +286,15 @@ class InvalidBundleSpecification(Excepti
>
>      This is used for syntax errors as opposed to support errors.
>      """
> +    __bytes__ = _tobytes
>
>  class UnsupportedBundleSpecification(Exception):
>      """error raised when a bundle specification is not supported."""
> +    __bytes__ = _tobytes
>
>  class CorruptedState(Exception):
>      """error raised when a command is not able to read its state from file"""
> +    __bytes__ = _tobytes
>
>  class PeerTransportError(Abort):
>      """Transport-level I/O error when communicating with a peer repo."""
> diff --git a/tests/test-py3-commands.t b/tests/test-py3-commands.t
> --- a/tests/test-py3-commands.t
> +++ b/tests/test-py3-commands.t
> @@ -151,6 +151,11 @@ Test weird unicode-vs-bytes stuff
>    options ([+] can be repeated):
>    (some details hidden, use --verbose to show complete help)
>
> +  $ $PYTHON3 $HGBIN help -k notopic
> +  abort: no matches
> +  (try 'hg help' for a list of topics)
> +  [255]
> +
>  Prove the repo is valid using the Python 2 `hg`:
>    $ hg verify
>    checking changesets
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Patch

diff --git a/mercurial/error.py b/mercurial/error.py
--- a/mercurial/error.py
+++ b/mercurial/error.py
@@ -13,7 +13,16 @@  imports.
 
 from __future__ import absolute_import
 
-# Do not import anything here, please
+# Do not import anything but pycompat here, please
+from . import pycompat
+
+def _tobytes(exc):
+    """Byte-stringify exception in the same way as BaseException_str()"""
+    if not exc.args:
+        return b''
+    if len(exc.args) == 1:
+        return pycompat.bytestr(exc.args[0])
+    return b'(%s)' % b', '.join(b"'%s'" % pycompat.bytestr(a) for a in exc.args)
 
 class Hint(object):
     """Mix-in to provide a hint of an error
@@ -26,10 +35,10 @@  class Hint(object):
         super(Hint, self).__init__(*args, **kw)
 
 class RevlogError(Hint, Exception):
-    pass
+    __bytes__ = _tobytes
 
 class FilteredIndexError(IndexError):
-    pass
+    __bytes__ = _tobytes
 
 class LookupError(RevlogError, KeyError):
     def __init__(self, name, index, message):
@@ -43,6 +52,9 @@  class LookupError(RevlogError, KeyError)
             name = short(name)
         RevlogError.__init__(self, '%s@%s: %s' % (index, name, message))
 
+    def __bytes__(self):
+        return RevlogError.__bytes__(self)
+
     def __str__(self):
         return RevlogError.__str__(self)
 
@@ -54,12 +66,15 @@  class ManifestLookupError(LookupError):
 
 class CommandError(Exception):
     """Exception raised on errors in parsing the command line."""
+    __bytes__ = _tobytes
 
 class InterventionRequired(Hint, Exception):
     """Exception raised when a command requires human intervention."""
+    __bytes__ = _tobytes
 
 class Abort(Hint, Exception):
     """Raised if a command needs to print an error and exit."""
+    __bytes__ = _tobytes
 
 class HookLoadError(Abort):
     """raised when loading a hook fails, aborting an operation
@@ -94,9 +109,11 @@  class ResponseExpected(Abort):
 
 class OutOfBandError(Hint, Exception):
     """Exception raised when a remote repo reports failure"""
+    __bytes__ = _tobytes
 
 class ParseError(Hint, Exception):
     """Raised when parsing config files and {rev,file}sets (msg[, pos])"""
+    __bytes__ = _tobytes
 
 class UnknownIdentifier(ParseError):
     """Exception raised when a {rev,file}set references an unknown identifier"""
@@ -108,7 +125,7 @@  class UnknownIdentifier(ParseError):
         self.symbols = symbols
 
 class RepoError(Hint, Exception):
-    pass
+    __bytes__ = _tobytes
 
 class RepoLookupError(RepoError):
     pass
@@ -128,6 +145,8 @@  class StdioError(IOError):
     def __init__(self, err):
         IOError.__init__(self, err.errno, err.strerror)
 
+    # no __bytes__() because error message is derived from the standard IOError
+
 class UnsupportedMergeRecords(Abort):
     def __init__(self, recordtypes):
         from .i18n import _
@@ -151,6 +170,8 @@  class LockError(IOError):
         IOError.__init__(self, errno, strerror, filename)
         self.desc = desc
 
+    # no __bytes__() because error message is derived from the standard IOError
+
 class LockHeld(LockError):
     def __init__(self, errno, filename, desc, locker):
         LockError.__init__(self, errno, 'Lock held', filename, desc)
@@ -161,36 +182,43 @@  class LockUnavailable(LockError):
 
 # LockError is for errors while acquiring the lock -- this is unrelated
 class LockInheritanceContractViolation(RuntimeError):
-    pass
+    __bytes__ = _tobytes
 
 class ResponseError(Exception):
     """Raised to print an error with part of output and exit."""
+    __bytes__ = _tobytes
 
 class UnknownCommand(Exception):
     """Exception raised if command is not in the command table."""
+    __bytes__ = _tobytes
 
 class AmbiguousCommand(Exception):
     """Exception raised if command shortcut matches more than one command."""
+    __bytes__ = _tobytes
 
 # derived from KeyboardInterrupt to simplify some breakout code
 class SignalInterrupt(KeyboardInterrupt):
     """Exception raised on SIGTERM and SIGHUP."""
 
 class SignatureError(Exception):
-    pass
+    __bytes__ = _tobytes
 
 class PushRaced(RuntimeError):
     """An exception raised during unbundling that indicate a push race"""
+    __bytes__ = _tobytes
 
 class ProgrammingError(Hint, RuntimeError):
     """Raised if a mercurial (core or extension) developer made a mistake"""
+    __bytes__ = _tobytes
 
 class WdirUnsupported(Exception):
     """An exception which is raised when 'wdir()' is not supported"""
+    __bytes__ = _tobytes
 
 # bundle2 related errors
 class BundleValueError(ValueError):
     """error raised when bundle2 cannot be processed"""
+    __bytes__ = _tobytes
 
 class BundleUnknownFeatureError(BundleValueError):
     def __init__(self, parttype=None, params=(), values=()):
@@ -217,6 +245,7 @@  class BundleUnknownFeatureError(BundleVa
 
 class ReadOnlyPartError(RuntimeError):
     """error raised when code tries to alter a part being generated"""
+    __bytes__ = _tobytes
 
 class PushkeyFailed(Abort):
     """error raised when a pushkey part failed to update a value"""
@@ -257,12 +286,15 @@  class InvalidBundleSpecification(Excepti
 
     This is used for syntax errors as opposed to support errors.
     """
+    __bytes__ = _tobytes
 
 class UnsupportedBundleSpecification(Exception):
     """error raised when a bundle specification is not supported."""
+    __bytes__ = _tobytes
 
 class CorruptedState(Exception):
     """error raised when a command is not able to read its state from file"""
+    __bytes__ = _tobytes
 
 class PeerTransportError(Abort):
     """Transport-level I/O error when communicating with a peer repo."""
diff --git a/tests/test-py3-commands.t b/tests/test-py3-commands.t
--- a/tests/test-py3-commands.t
+++ b/tests/test-py3-commands.t
@@ -151,6 +151,11 @@  Test weird unicode-vs-bytes stuff
   options ([+] can be repeated):
   (some details hidden, use --verbose to show complete help)
 
+  $ $PYTHON3 $HGBIN help -k notopic
+  abort: no matches
+  (try 'hg help' for a list of topics)
+  [255]
+
 Prove the repo is valid using the Python 2 `hg`:
   $ hg verify
   checking changesets