From 057905fa1d074e6dd341822e5a6a1e49b6b97a21 Mon Sep 17 00:00:00 2001 From: Mike Frysinger Date: Thu, 18 Feb 2021 23:28:32 -0500 Subject: [PATCH] error: fix pickling of all exceptions Make sure all our custom exceptions can be pickled so that if they get thrown in a multiprocess subprocess, we don't crash & hang due to multiprocessing being unable to pickle+unpickle the exception. Details/examples can be seen in Python reports like: https://bugs.python.org/issue13751 Change-Id: Iddf14d3952ad4e2867cfc71891d6b6559130df4b Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/297382 Reviewed-by: Michael Mortensen Tested-by: Mike Frysinger --- error.py | 16 +++++++------- tests/test_error.py | 53 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 61 insertions(+), 8 deletions(-) create mode 100644 tests/test_error.py diff --git a/error.py b/error.py index 8bb64b8f..20d5f725 100644 --- a/error.py +++ b/error.py @@ -37,7 +37,7 @@ class NoManifestException(Exception): """ def __init__(self, path, reason): - super(NoManifestException, self).__init__() + super(NoManifestException, self).__init__(path, reason) self.path = path self.reason = reason @@ -50,7 +50,7 @@ class EditorError(Exception): """ def __init__(self, reason): - super(EditorError, self).__init__() + super(EditorError, self).__init__(reason) self.reason = reason def __str__(self): @@ -62,7 +62,7 @@ class GitError(Exception): """ def __init__(self, command): - super(GitError, self).__init__() + super(GitError, self).__init__(command) self.command = command def __str__(self): @@ -74,7 +74,7 @@ class UploadError(Exception): """ def __init__(self, reason): - super(UploadError, self).__init__() + super(UploadError, self).__init__(reason) self.reason = reason def __str__(self): @@ -86,7 +86,7 @@ class DownloadError(Exception): """ def __init__(self, reason): - super(DownloadError, self).__init__() + super(DownloadError, self).__init__(reason) self.reason = reason def __str__(self): @@ -98,7 +98,7 @@ class NoSuchProjectError(Exception): """ def __init__(self, name=None): - super(NoSuchProjectError, self).__init__() + super(NoSuchProjectError, self).__init__(name) self.name = name def __str__(self): @@ -112,7 +112,7 @@ class InvalidProjectGroupsError(Exception): """ def __init__(self, name=None): - super(InvalidProjectGroupsError, self).__init__() + super(InvalidProjectGroupsError, self).__init__(name) self.name = name def __str__(self): @@ -128,7 +128,7 @@ class RepoChangedException(Exception): """ def __init__(self, extra_args=None): - super(RepoChangedException, self).__init__() + super(RepoChangedException, self).__init__(extra_args) self.extra_args = extra_args or [] diff --git a/tests/test_error.py b/tests/test_error.py new file mode 100644 index 00000000..82b00c24 --- /dev/null +++ b/tests/test_error.py @@ -0,0 +1,53 @@ +# Copyright 2021 The Android Open Source Project +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""Unittests for the error.py module.""" + +import inspect +import pickle +import unittest + +import error + + +class PickleTests(unittest.TestCase): + """Make sure all our custom exceptions can be pickled.""" + + def getExceptions(self): + """Return all our custom exceptions.""" + for name in dir(error): + cls = getattr(error, name) + if isinstance(cls, type) and issubclass(cls, Exception): + yield cls + + def testExceptionLookup(self): + """Make sure our introspection logic works.""" + classes = list(self.getExceptions()) + self.assertIn(error.HookError, classes) + # Don't assert the exact number to avoid being a change-detector test. + self.assertGreater(len(classes), 10) + + def testPickle(self): + """Try to pickle all the exceptions.""" + for cls in self.getExceptions(): + args = inspect.getfullargspec(cls.__init__).args[1:] + obj = cls(*args) + p = pickle.dumps(obj) + try: + newobj = pickle.loads(p) + except Exception as e: # pylint: disable=broad-except + self.fail('Class %s is unable to be pickled: %s\n' + 'Incomplete super().__init__(...) call?' % (cls, e)) + self.assertIsInstance(newobj, cls) + self.assertEqual(str(obj), str(newobj))