From 0b284db33a84bca46b2127ad20d80ba912535add Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Tue, 15 Apr 2025 18:09:00 +0200 Subject: [PATCH 1/2] fix remote repository exception handling / exit codes, fixes #8631 also: reduce code duplication. --- src/borg/remote.py | 30 +++++++++++++++---- .../testsuite/archiver/return_codes_test.py | 18 ++++++++++- 2 files changed, 41 insertions(+), 7 deletions(-) diff --git a/src/borg/remote.py b/src/borg/remote.py index 7c0871c02..84cfe0b94 100644 --- a/src/borg/remote.py +++ b/src/borg/remote.py @@ -265,6 +265,19 @@ class RepositoryServer: # pragma: no cover args = self.filter_args(f, args) res = f(**args) except BaseException as e: + # These exceptions are reconstructed on the client end in RemoteRepository.call_many(), + # and will be handled just like locally raised exceptions. Suppress the remote traceback + # for these, except ErrorWithTraceback, which should always display a traceback. + reconstructed_exceptions = ( + Repository.InvalidRepository, + Repository.InvalidRepositoryConfig, + Repository.DoesNotExist, + Repository.AlreadyExists, + Repository.PathAlreadyExists, + PathNotAllowed, + Repository.InsufficientFreeSpaceError, + Repository.StorageQuotaExceeded, + ) # logger.exception(e) ex_short = traceback.format_exception_only(e.__class__, e) ex_full = traceback.format_exception(*sys.exc_info()) @@ -272,12 +285,7 @@ class RepositoryServer: # pragma: no cover if isinstance(e, Error): ex_short = [e.get_message()] ex_trace = e.traceback - if isinstance(e, (self.RepoCls.DoesNotExist, self.RepoCls.AlreadyExists, PathNotAllowed)): - # These exceptions are reconstructed on the client end in RemoteRepository*.call_many(), - # and will be handled just like locally raised exceptions. Suppress the remote traceback - # for these, except ErrorWithTraceback, which should always display a traceback. - pass - else: + if not isinstance(e, reconstructed_exceptions): logging.debug("\n".join(ex_full)) sys_info = sysinfo() @@ -790,6 +798,8 @@ class RemoteRepository: raise Error(args[0]) elif error == "ErrorWithTraceback": raise ErrorWithTraceback(args[0]) + elif error == "InvalidRepository": + raise Repository.InvalidRepository(self.location.processed) elif error == "DoesNotExist": raise Repository.DoesNotExist(self.location.processed) elif error == "AlreadyExists": @@ -802,6 +812,8 @@ class RemoteRepository: raise PathNotAllowed(args[0]) elif error == "PathPermissionDenied": raise Repository.PathPermissionDenied(args[0]) + elif error == "PathAlreadyExists": + raise Repository.PathAlreadyExists(args[0]) elif error == "ParentPathDoesNotExist": raise Repository.ParentPathDoesNotExist(args[0]) elif error == "ObjectNotFound": @@ -818,6 +830,12 @@ class RemoteRepository: raise NotMyLock(args[0]) elif error == "NoManifestError": raise NoManifestError + elif error == "InsufficientFreeSpaceError": + raise Repository.InsufficientFreeSpaceError(args[0], args[1]) + elif error == "InvalidRepositoryConfig": + raise Repository.InvalidRepositoryConfig(self.location.processed, args[1]) + elif error == "StorageQuotaExceeded": + raise Repository.StorageQuotaExceeded(args[0], args[1]) else: raise self.RPCError(unpacked) diff --git a/src/borg/testsuite/archiver/return_codes_test.py b/src/borg/testsuite/archiver/return_codes_test.py index dfbbac3ef..dbd159735 100644 --- a/src/borg/testsuite/archiver/return_codes_test.py +++ b/src/borg/testsuite/archiver/return_codes_test.py @@ -1,6 +1,11 @@ +import os + from ...constants import * # NOQA from ...helpers import IncludePatternNeverMatchedWarning -from . import cmd_fixture, changedir # NOQA +from ...repository import Repository +from . import cmd, cmd_fixture, changedir, generate_archiver_tests # NOQA + +pytest_generate_tests = lambda metafunc: generate_archiver_tests(metafunc, kinds="local,remote,binary") # NOQA def test_return_codes(cmd_fixture, tmpdir): @@ -17,3 +22,14 @@ def test_return_codes(cmd_fixture, tmpdir): assert rc == EXIT_SUCCESS rc, out = cmd_fixture("--repo=%s" % repo, "extract", "archive", "does/not/match") assert rc == IncludePatternNeverMatchedWarning().exit_code + + +def test_exit_codes(archivers, request, tmpdir, monkeypatch): + archiver = request.getfixturevalue(archivers) + # we create the repo path, but do NOT initialize the borg repo, + # so the borg create commands are expected to fail with DoesNotExist (was: InvalidRepository in borg 1.4). + os.makedirs(archiver.repository_path) + monkeypatch.setenv("BORG_EXIT_CODES", "classic") + cmd(archiver, "create", "archive", "input", fork=True, exit_code=EXIT_ERROR) + monkeypatch.setenv("BORG_EXIT_CODES", "modern") + cmd(archiver, "create", "archive", "input", fork=True, exit_code=Repository.DoesNotExist.exit_mcode) From 8ec48ffb0560baceb0e9750527a1cb5497ba8e47 Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Thu, 17 Apr 2025 17:57:38 +0200 Subject: [PATCH 2/2] refactor test_return_codes ... so works in a similar way as test_exit_codes. --- .../testsuite/archiver/return_codes_test.py | 32 +++++++++---------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/src/borg/testsuite/archiver/return_codes_test.py b/src/borg/testsuite/archiver/return_codes_test.py index dbd159735..5ac4861c5 100644 --- a/src/borg/testsuite/archiver/return_codes_test.py +++ b/src/borg/testsuite/archiver/return_codes_test.py @@ -3,28 +3,28 @@ import os from ...constants import * # NOQA from ...helpers import IncludePatternNeverMatchedWarning from ...repository import Repository -from . import cmd, cmd_fixture, changedir, generate_archiver_tests # NOQA +from . import cmd, changedir, generate_archiver_tests # NOQA pytest_generate_tests = lambda metafunc: generate_archiver_tests(metafunc, kinds="local,remote,binary") # NOQA -def test_return_codes(cmd_fixture, tmpdir): - repo = tmpdir / "repo" # borg creates the directory - input = tmpdir.mkdir("input") - output = tmpdir.mkdir("output") - input.join("test_file").write("content") - rc, out = cmd_fixture("--repo=%s" % str(repo), "repo-create", "--encryption=none") - assert rc == EXIT_SUCCESS - rc, out = cmd_fixture("--repo=%s" % repo, "create", "archive", str(input)) - assert rc == EXIT_SUCCESS - with changedir(str(output)): - rc, out = cmd_fixture("--repo=%s" % repo, "extract", "archive") - assert rc == EXIT_SUCCESS - rc, out = cmd_fixture("--repo=%s" % repo, "extract", "archive", "does/not/match") - assert rc == IncludePatternNeverMatchedWarning().exit_code +def test_return_codes(archivers, request): + archiver = request.getfixturevalue(archivers) + cmd(archiver, "repo-create", "--encryption=none") + cmd(archiver, "create", "archive", "input") + with changedir("output"): + cmd(archiver, "extract", "archive") + cmd( + archiver, + "extract", + "archive", + "does/not/match", + fork=True, + exit_code=IncludePatternNeverMatchedWarning().exit_code, + ) -def test_exit_codes(archivers, request, tmpdir, monkeypatch): +def test_exit_codes(archivers, request, monkeypatch): archiver = request.getfixturevalue(archivers) # we create the repo path, but do NOT initialize the borg repo, # so the borg create commands are expected to fail with DoesNotExist (was: InvalidRepository in borg 1.4).