Report errors of jsondiff and jsonpatch in one line

doc/commandline.rst

@@ -7,6 +7,11 @@ The JSON patch package contains the commandline utilities ``jsondiff`` and
 Both utilities read JSON files as UTF-8. When ``jsonpatch`` modifies a file
 in-place, it also writes UTF-8.
 
+If a file cannot be read or written, is not valid JSON, or if the patch
+cannot be applied, both print a one-line error message to standard error and
+exit with status 2. ``jsondiff`` exits with status 1 if the files differ,
+like ``diff``, and with 0 if they are equal.
+
 ``jsondiff``
 ------------
 

jsonpatch_cli.py

@@ -34,19 +34,38 @@ def _input_files(*streams):
     return inputs
 
 
-def jsondiff_main():
+def _load(stream, loads=json.loads):
+    """ Parses the JSON document in stream; errors name the file """
     try:
-        diff_files()
+        return loads(stream.read())
+    except ValueError as ex:
+        # invalid JSON or, as a UnicodeDecodeError, invalid UTF-8
+        name = getattr(stream, 'name', '<input>')
+        raise ValueError('{0}: {1}'.format(name, ex)) from ex
+
+
+def _run(parser, program):
+    """ Runs a command line program. Errors in its input, and in reading
+    and writing files, are reported in one line, with exit status 2 """
+    try:
+        program()
     except KeyboardInterrupt:
         sys.exit(1)
+    except (jsonpatch.JsonPatchException, jsonpatch.JsonPointerException,
+            ValueError, OSError) as ex:
+        parser.exit(2, '{0}: error: {1}\n'.format(parser.prog, ex))
+
+
+def jsondiff_main():
+    _run(jsondiff_parser, diff_files)
 
 
 def diff_files():
     """ Diffs two JSON files and prints a patch """
     args = jsondiff_parser.parse_args()
     with _input_files(args.FILE1, args.FILE2):
-        doc1 = json.load(args.FILE1)
-        doc2 = json.load(args.FILE2)
+        doc1 = _load(args.FILE1)
+        doc2 = _load(args.FILE2)
     patch = jsonpatch.make_patch(doc1, doc2, generate_test_ops=args.test_ops)
     if patch.patch:
         print(json.dumps(patch.patch, indent=args.indent, ensure_ascii=not args.preserve_unicode))
@@ -74,10 +93,7 @@ def diff_files():
 
 
 def jsonpatch_main():
-    try:
-        patch_files()
-    except KeyboardInterrupt:
-        sys.exit(1)
+    _run(jsonpatch_parser, patch_files)
 
 
 def patch_files():
@@ -86,8 +102,9 @@ def patch_files():
     # The inputs are closed before anything is written, as Windows cannot
     # replace ORIGINAL while it is open.
     with _input_files(args.ORIGINAL, args.PATCH):
-        doc = json.load(args.ORIGINAL)
-        result = jsonpatch.apply_patch(doc, args.PATCH.read())
+        doc = _load(args.ORIGINAL)
+        patch = _load(args.PATCH, jsonpatch.JsonPatch.from_string)
+    result = patch.apply(doc)
 
     if args.in_place:
         dirname = os.path.abspath(os.path.dirname(args.ORIGINAL.name))

tests.py

@@ -3230,6 +3230,100 @@ def test_jsondiff_equal_files(self):
         res = self.run_script('jsondiff', src, dst)
         self.assertEqual((res.returncode, res.stdout), (0, ''), res.stderr)
 
+    def assert_error(self, res, program, *parts):
+        """ The program failed with a one-line message containing parts """
+        self.assertEqual(res.returncode, 2, res.stderr)
+        self.assertEqual(res.stdout, '')
+        self.assertEqual(res.stderr.count('\n'), 1, res.stderr)
+        self.assertTrue(res.stderr.startswith(program + ': error: '), res.stderr)
+        for part in parts:
+            self.assertIn(part, res.stderr)
+
+    def test_jsondiff_reports_invalid_json(self):
+        src = self.write('a.json', {'a': 1})
+        dst = os.path.join(self.dir, 'b.json')
+        with open(dst, 'w', encoding='utf-8') as f:
+            f.write('{"a": ')
+        for files in ((src, dst), (dst, src)):
+            with self.subTest(files=files):
+                res = self.run_script('jsondiff', *files)
+                self.assert_error(res, 'jsondiff', 'b.json', 'line 1 column 7')
+
+    def test_jsondiff_reports_invalid_utf8(self):
+        src = self.write('a.json', {'a': 1})
+        dst = os.path.join(self.dir, 'b.json')
+        with open(dst, 'wb') as f:
+            f.write(b'"\xff"')
+        res = self.run_script('jsondiff', src, dst)
+        self.assert_error(res, 'jsondiff', 'b.json', 'utf-8')
+
+    def test_jsonpatch_reports_failing_patch(self):
+        doc = self.write('doc.json', {'a': 1})
+        cases = [
+            ([{'op': 'remove', 'path': '/b'}], "can't remove a non-existent object 'b'"),
+            ([{'op': 'test', 'path': '/a', 'value': 2}], 'is not equal to tested value'),
+            ([{'op': 'remove', 'path': '/b/c'}], "member 'b' not found"),
+            ([{'op': 'frobnicate', 'path': '/a'}], "Unknown operation 'frobnicate'"),
+            ({'op': 'remove', 'path': '/a'}, 'sequence of operations'),
+        ]
+        for patch, message in cases:
+            with self.subTest(patch=patch):
+                res = self.run_script('jsonpatch', doc, self.write('patch.json', patch))
+                self.assert_error(res, 'jsonpatch', message)
+
+    def test_jsonpatch_reports_invalid_json(self):
+        doc = self.write('doc.json', {'a': 1})
+        patch = self.write('patch.json', [])
+        broken = os.path.join(self.dir, 'broken.json')
+        with open(broken, 'w', encoding='utf-8') as f:
+            f.write('[')
+        res = self.run_script('jsonpatch', broken, patch)
+        self.assert_error(res, 'jsonpatch', 'broken.json', 'line 1 column 2')
+        res = self.run_script('jsonpatch', doc, broken)
+        self.assert_error(res, 'jsonpatch', 'broken.json', 'line 1 column 2')
+        res = self.run_script('jsonpatch', doc, stdin='[')
+        self.assert_error(res, 'jsonpatch', '<stdin>', 'line 1 column 2')
+
+    def test_jsonpatch_reports_invalid_json_of_unnamed_stream(self):
+        doc = self.write('doc.json', {'a': 1})
+        parser = jsonpatch_cli.jsonpatch_parser
+        args = parser.parse_args([doc])
+        self.addCleanup(args.ORIGINAL.close)
+        # a stream without a name, as sys.stdin can be replaced by
+        args.PATCH = io.StringIO('[')
+        with mock.patch.object(parser, 'parse_args', return_value=args), \
+                contextlib.redirect_stderr(io.StringIO()) as stderr:
+            with self.assertRaises(SystemExit) as caught:
+                jsonpatch_cli.jsonpatch_main()
+        self.assertEqual(caught.exception.code, 2)
+        self.assertTrue(stderr.getvalue().startswith('jsonpatch: error: <input>: '),
+                        stderr.getvalue())
+
+    def test_jsonpatch_reports_output_error(self):
+        doc = self.write('doc.json', {'a': 1})
+        patch = self.write('patch.json', [])
+        failure = PermissionError(13, 'Permission denied', 'doc.json')
+        with mock.patch('sys.argv', ['jsonpatch', '-i', doc, patch]), \
+                mock.patch('os.replace', side_effect=failure), \
+                contextlib.redirect_stderr(io.StringIO()) as stderr:
+            with self.assertRaises(SystemExit) as caught:
+                jsonpatch_cli.jsonpatch_main()
+        self.assertEqual(caught.exception.code, 2)
+        self.assertEqual(stderr.getvalue(),
+                         "jsonpatch: error: [Errno 13] Permission denied: 'doc.json'\n")
+        self.assertEqual(self.read(doc), {'a': 1})
+
+    def test_interrupt_is_not_reported(self):
+        for main, name in ((jsonpatch_cli.jsondiff_main, 'diff_files'),
+                           (jsonpatch_cli.jsonpatch_main, 'patch_files')):
+            with self.subTest(program=name):
+                with mock.patch.object(jsonpatch_cli, name, side_effect=KeyboardInterrupt), \
+                        contextlib.redirect_stderr(io.StringIO()) as stderr:
+                    with self.assertRaises(SystemExit) as caught:
+                        main()
+                self.assertEqual(caught.exception.code, 1)
+                self.assertEqual(stderr.getvalue(), '')
+
     def test_jsondiff_closes_inputs(self):
         for target in (1, 2):
             with self.subTest(target=target):
@@ -3265,8 +3359,10 @@ def test_jsondiff_closes_both_inputs_after_parse_error(self):
                 self.addCleanup(args.FILE1.close)
                 self.addCleanup(args.FILE2.close)
                 with mock.patch.object(parser, 'parse_args', return_value=args):
-                    with self.assertRaises(json.JSONDecodeError):
+                    with self.assertRaises(ValueError) as caught:
                         jsonpatch_cli.diff_files()
+                self.assertIsInstance(caught.exception.__cause__, json.JSONDecodeError)
+                self.assertIn(os.path.basename(paths[invalid]), str(caught.exception))
                 self.assertTrue(args.FILE1.closed)
                 self.assertTrue(args.FILE2.closed)
 
@@ -3381,8 +3477,7 @@ def test_jsonpatch_rejects_invalid_patch_text_before_in_place_write(self):
                         with open(path, 'w', encoding='utf-8') as f:
                             f.write(patch)
                         res = self.run_script('jsonpatch', '-i', doc, path)
-                    self.assertNotEqual(res.returncode, 0)
-                    self.assertIn('InvalidJsonPatch', res.stderr)
+                    self.assert_error(res, 'jsonpatch')
                     with open(doc, 'rb') as f:
                         self.assertEqual(f.read(), original)
 
@@ -3611,8 +3706,7 @@ def test_jsonpatch_in_place_failing_patch(self):
         doc = self.write('doc.json', {'a': 1})
         patch = self.write('patch.json', [{'op': 'remove', 'path': '/b'}])
         res = self.run_script('jsonpatch', '-i', '-b', doc, patch)
-        self.assertNotEqual(res.returncode, 0)
-        self.assertIn('JsonPatchPathNotFound', res.stderr)
+        self.assert_error(res, 'jsonpatch', "can't remove a non-existent object 'b'")
         self.assertEqual(self.read(doc), {'a': 1})
         self.assertEqual(sorted(os.listdir(self.dir)), ['doc.json', 'patch.json'])