From ad7dcb461579afebcf01e7da4f344f255c55d3d9 Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Fri, 5 Apr 2024 12:23:50 -0700 Subject: [PATCH] Fix "--json" error when Borg includes non-JSON warnings in JSON output (#847). --- NEWS | 1 + borgmatic/actions/json.py | 17 ++++++++++++++++- tests/unit/actions/test_json.py | 19 +++++++++++++------ 3 files changed, 30 insertions(+), 7 deletions(-) diff --git a/NEWS b/NEWS index 4eb3ec2..744e18c 100644 --- a/NEWS +++ b/NEWS @@ -3,6 +3,7 @@ configured monitoring hooks. * #843: Add documentation link to Loki dashboard for borgmatic: https://torsion.org/borgmatic/docs/how-to/monitor-your-backups/#loki-hook + * #847: Fix "--json" error when Borg includes non-JSON warnings in JSON output. * Fix handling of the NO_COLOR environment variable to ignore an empty value. * Add documentation about backing up containerized databases by configuring borgmatic to exec into a container to run a dump command: diff --git a/borgmatic/actions/json.py b/borgmatic/actions/json.py index cf849f2..0a71166 100644 --- a/borgmatic/actions/json.py +++ b/borgmatic/actions/json.py @@ -1,12 +1,27 @@ +import logging import json +logger = logging.getLogger(__name__) + + def parse_json(borg_json_output, label): ''' Given a Borg JSON output string, parse it as JSON into a dict. Inject the given borgmatic repository label into it and return the dict. + + Raise JSONDecodeError if the JSON output cannot be parsed. ''' - json_data = json.loads(borg_json_output) + lines = borg_json_output.splitlines() + start_line_index = 0 + + # Scan forward to find the first line starting with "{" and assume that's where the JSON starts. + for line_index, line in enumerate(lines): + if line.startswith('{'): + start_line_index = line_index + break + + json_data = json.loads('\n'.join(lines[start_line_index:])) if 'repository' not in json_data: return json_data diff --git a/tests/unit/actions/test_json.py b/tests/unit/actions/test_json.py index 9f7f839..3908286 100644 --- a/tests/unit/actions/test_json.py +++ b/tests/unit/actions/test_json.py @@ -1,25 +1,32 @@ +import pytest from flexmock import flexmock from borgmatic.actions import json as module def test_parse_json_loads_json_from_string(): - flexmock(module.json).should_receive('loads').and_return({'repository': {'id': 'foo'}}) - assert module.parse_json('{"repository": {"id": "foo"}}', label=None) == { 'repository': {'id': 'foo', 'label': ''} } -def test_parse_json_injects_label_into_parsed_data(): - flexmock(module.json).should_receive('loads').and_return({'repository': {'id': 'foo'}}) +def test_parse_json_skips_non_json_warnings_and_loads_subsequent_json(): + assert module.parse_json( + '/non/existent/path: stat: [Errno 2] No such file or directory: /non/existent/path\n{"repository":\n{"id": "foo"}}', + label=None, + ) == {'repository': {'id': 'foo', 'label': ''}} + +def test_parse_json_skips_with_invalid_json_raises(): + with pytest.raises(module.json.JSONDecodeError): + module.parse_json('this is not valid JSON }', label=None) + + +def test_parse_json_injects_label_into_parsed_data(): assert module.parse_json('{"repository": {"id": "foo"}}', label='bar') == { 'repository': {'id': 'foo', 'label': 'bar'} } def test_parse_json_injects_nothing_when_repository_missing(): - flexmock(module.json).should_receive('loads').and_return({'stuff': {'id': 'foo'}}) - assert module.parse_json('{"stuff": {"id": "foo"}}', label='bar') == {'stuff': {'id': 'foo'}}