Try to give a more informative error as this is user facing (bugfix) (#2353)

* Try to give a more informative error as this is user facing

The previous error gave basically no reason why the json was invalid.
This improves the situation adding exactly what the json decoder thinks
is invalid with a primitive ^ hightlight

* Refactor into independent function and tests

* Also test binary docs

* Bytes are not supported on 3.5 as straight input

bytes end up in the doc only when parsing a file and I cant
be bothered to test that

Author: Hook25

checkbox-ng/plainbox/impl/exporter/jinja2.py

@@ -52,7 +52,6 @@
 from plainbox.impl.unit.exporter import ExporterError
 from plainbox.impl.config import CheckboxINIParser
 
-
 #: Name-space prefix for Canonical Certification
 CERTIFICATION_NS = "com.canonical.certification::"
 
@@ -83,6 +82,25 @@ def highlight_keys(text):
     return re.sub(r"(\w+:\s)", r"<b>\1</b>", text)
 
 
+def pretty_json_decode_error(
+    error: json.decoder.JSONDecodeError, lines_around=3
+):
+    lineno = error.lineno
+    colno = error.colno
+    lines = error.doc.splitlines()
+    min_line = max(lineno - lines_around, 0)
+    max_line = min(lineno + lines_around, len(lines) - 1)
+    center = min(lineno + 1, len(lines))
+    error_repr = lines[min_line:center]
+    error_repr.append(" " * colno + "^^^ " + str(error))
+    error_repr += lines[center:max_line]
+    if isinstance(error_repr[0], str):
+        return "\n".join(error_repr)
+    else:
+        # defer decoding here to avoid decoding the whole document
+        return b"\n".join(error_repr).decode("utf8")
+
+
 class Jinja2SessionStateExporter(SessionStateExporterBase):
     """Session state exporter that renders output using jinja2 template."""
 
@@ -279,5 +297,7 @@ def validate_json(self, stream):
             s = raw.decode("utf-8") if type(raw) == bytes else raw
             json.loads(s)
             return []
+        except json.decoder.JSONDecodeError as exc:
+            return pretty_json_decode_error(exc)
         except Exception as exc:
             return [str(exc)]

checkbox-ng/plainbox/impl/exporter/test_jinja2.py

@@ -27,9 +27,11 @@
 from tempfile import TemporaryDirectory
 from textwrap import dedent
 from unittest import TestCase
+import json
 import os
 
 from plainbox.impl.exporter.jinja2 import Jinja2SessionStateExporter
+from plainbox.impl.exporter.jinja2 import pretty_json_decode_error
 from plainbox.impl.result import MemoryJobResult
 from plainbox.impl.session.state import SessionMetaData
 from plainbox.impl.unit.exporter import ExporterError
@@ -165,3 +167,55 @@ def test_validation_json_throws(self):
                 exporter.dump_from_session_manager(
                     self.manager_single_job, stream
                 )
+
+
+class PrettyJsonDecodeErrorTests(TestCase):
+
+    def _get_error(self, bad_json):
+        try:
+            json.loads(bad_json)
+        except json.decoder.JSONDecodeError as e:
+            return e
+        self.fail("Expected JSONDecodeError was not raised")
+
+    def _check(self, bad_json):
+        error = self._get_error(bad_json)
+        result = pretty_json_decode_error(error)
+        self.assertIsInstance(result, str)
+        # The original error message should always be present
+        self.assertIn(str(error), result)
+        return result
+
+    def test_error_at_start(self):
+        result = self._check("not json at all")
+        self.assertIn("not json", result)
+
+    def test_trailing_comma_in_object(self):
+        self._check('{"key": "value",}')
+
+    def test_trailing_comma_in_array(self):
+        self._check("[1, 2, 3,]")
+
+    def test_missing_colon(self):
+        self._check('{"key" "value"}')
+
+    def test_single_quotes(self):
+        self._check("{'key': 'value'}")
+
+    def test_empty_string(self):
+        self._check("")
+
+    def test_truncated_object(self):
+        self._check('{"key": "value"')
+
+    def test_error_deep_in_long_document(self):
+        bad_json = '{"a": 1, ' * 100 + "BROKEN}"
+        self._check(bad_json)
+
+    def test_multiline_json(self):
+        self._check('{\n  "a": 1,\n  "b": ,\n  "c": 3\n}')
+
+    def test_result_contains_error_neighbourhood(self):
+        bad_json = '{"good": 1, "bad": tralse, "other": 2}'
+        result = self._check(bad_json)
+        self.assertIn("tralse", result)