소스 검색

Make user-facing manifest loading error messages a little friendlier (#697).

Dan Helfman 2 년 전
부모
커밋
beb899d6fb
3개의 변경된 파일94개의 추가작업 그리고 5개의 파일을 삭제
  1. 22 2
      borgmatic/actions/config/bootstrap.py
  2. 1 3
      borgmatic/commands/borgmatic.py
  3. 71 0
      tests/unit/actions/config/test_bootstrap.py

+ 22 - 2
borgmatic/actions/config/bootstrap.py

@@ -21,6 +21,9 @@ def get_config_paths(bootstrap_arguments, global_arguments, local_borg_version):
     Return:
     Return:
     The config paths from the manifest.json file in the borgmatic source directory after extracting it from the
     The config paths from the manifest.json file in the borgmatic source directory after extracting it from the
     repository.
     repository.
+
+    Raise ValueError if the manifest JSON is missing, can't be decoded, or doesn't contain the
+    expected configuration path data.
     '''
     '''
     borgmatic_source_directory = (
     borgmatic_source_directory = (
         bootstrap_arguments.borgmatic_source_directory or DEFAULT_BORGMATIC_SOURCE_DIRECTORY
         bootstrap_arguments.borgmatic_source_directory or DEFAULT_BORGMATIC_SOURCE_DIRECTORY
@@ -46,14 +49,31 @@ def get_config_paths(bootstrap_arguments, global_arguments, local_borg_version):
         extract_to_stdout=True,
         extract_to_stdout=True,
     )
     )
 
 
-    manifest_data = json.loads(extract_process.stdout.read())
+    manifest_json = extract_process.stdout.read()
+    if not manifest_json:
+        raise ValueError(
+            'Cannot read configuration paths from archive due to missing bootstrap manifest'
+        )
 
 
-    return manifest_data['config_paths']
+    try:
+        manifest_data = json.loads(manifest_json)
+    except json.JSONDecodeError as error:
+        raise ValueError(
+            f'Cannot read configuration paths from archive due to invalid bootstrap manifest JSON: {error}'
+        )
+
+    try:
+        return manifest_data['config_paths']
+    except KeyError:
+        raise ValueError('Cannot read configuration paths from archive due to invalid bootstrap manifest')
 
 
 
 
 def run_bootstrap(bootstrap_arguments, global_arguments, local_borg_version):
 def run_bootstrap(bootstrap_arguments, global_arguments, local_borg_version):
     '''
     '''
     Run the "bootstrap" action for the given repository.
     Run the "bootstrap" action for the given repository.
+
+    Raise ValueError if the bootstrap configuration could not be loaded.
+    Raise CalledProcessError or OSError if Borg could not be run.
     '''
     '''
     manifest_config_paths = get_config_paths(
     manifest_config_paths = get_config_paths(
         bootstrap_arguments, global_arguments, local_borg_version
         bootstrap_arguments, global_arguments, local_borg_version

+ 1 - 3
borgmatic/commands/borgmatic.py

@@ -647,10 +647,8 @@ def collect_configuration_run_summary_logs(configs, arguments):
             CalledProcessError,
             CalledProcessError,
             ValueError,
             ValueError,
             OSError,
             OSError,
-            json.JSONDecodeError,
-            KeyError,
         ) as error:
         ) as error:
-            yield from log_error_records('Error running bootstrap', error)
+            yield from log_error_records(error)
 
 
         return
         return
 
 

+ 71 - 0
tests/unit/actions/config/test_bootstrap.py

@@ -1,3 +1,4 @@
+import pytest
 from flexmock import flexmock
 from flexmock import flexmock
 
 
 from borgmatic.actions.config import bootstrap as module
 from borgmatic.actions.config import bootstrap as module
@@ -29,6 +30,76 @@ def test_get_config_paths_returns_list_of_config_paths():
     ]
     ]
 
 
 
 
+def test_get_config_paths_with_missing_manifest_raises_value_error():
+    bootstrap_arguments = flexmock(
+        borgmatic_source_directory=None,
+        repository='repo',
+        archive='archive',
+    )
+    global_arguments = flexmock(
+        dry_run=False,
+    )
+    local_borg_version = flexmock()
+    extract_process = flexmock(stdout=flexmock(read=lambda: ''))
+    flexmock(module.borgmatic.borg.extract).should_receive('extract_archive').and_return(
+        extract_process
+    )
+    flexmock(module.borgmatic.borg.rlist).should_receive('resolve_archive_name').and_return(
+        'archive'
+    )
+
+    with pytest.raises(ValueError):
+        module.get_config_paths(bootstrap_arguments, global_arguments, local_borg_version)
+
+
+def test_get_config_paths_with_broken_json_raises_value_error():
+    bootstrap_arguments = flexmock(
+        borgmatic_source_directory=None,
+        repository='repo',
+        archive='archive',
+    )
+    global_arguments = flexmock(
+        dry_run=False,
+    )
+    local_borg_version = flexmock()
+    extract_process = flexmock(
+        stdout=flexmock(read=lambda: '{"config_paths": ["/oops'),
+    )
+    flexmock(module.borgmatic.borg.extract).should_receive('extract_archive').and_return(
+        extract_process
+    )
+    flexmock(module.borgmatic.borg.rlist).should_receive('resolve_archive_name').and_return(
+        'archive'
+    )
+
+    with pytest.raises(ValueError):
+        module.get_config_paths(bootstrap_arguments, global_arguments, local_borg_version)
+
+
+def test_get_config_paths_with_json_missing_key_raises_value_error():
+    bootstrap_arguments = flexmock(
+        borgmatic_source_directory=None,
+        repository='repo',
+        archive='archive',
+    )
+    global_arguments = flexmock(
+        dry_run=False,
+    )
+    local_borg_version = flexmock()
+    extract_process = flexmock(
+        stdout=flexmock(read=lambda: '{}'),
+    )
+    flexmock(module.borgmatic.borg.extract).should_receive('extract_archive').and_return(
+        extract_process
+    )
+    flexmock(module.borgmatic.borg.rlist).should_receive('resolve_archive_name').and_return(
+        'archive'
+    )
+
+    with pytest.raises(ValueError):
+        module.get_config_paths(bootstrap_arguments, global_arguments, local_borg_version)
+
+
 def test_run_bootstrap_does_not_raise():
 def test_run_bootstrap_does_not_raise():
     bootstrap_arguments = flexmock(
     bootstrap_arguments = flexmock(
         repository='repo',
         repository='repo',