Separate error offset from error message

[finitemonkey]

Hi! Thank you for all your work on this library, it is working well for me but I had a problem with exception error messages not giving enough insight.

The person who raised issue #26 wanted the errors to convey more detail about the nature of the error, but in my case I really just needed to get a better idea of where we are in the JSON as I was having difficulty quickly translating "At offset: 357" into something meaningful.

You said in #26 that although better messages would be nice, you do not want to keep any additional state as it would slow down parsing (fair enough)... so the idea of this PR is that if the offset value is separated from the message part of the error, into its own var, it would allow a proc to be written to reconstruct the offset info into a more human-friendly form. A proc something like this;

import strutils

proc prettyPrintJsonError(json: string, error: ref JsonError) =
  var newlinePreError = 0
  var newlinePostError = json.len - 1

  for i in countdown(error.offset - 1, 0):
    if json[i] == '\n':
      newlinePreError = i
      break

  for i in countup(error.offset, json.len - 1):
    if json[i] == '\n':
      newlinePostError = i
      break

  let indent = spaces(max(0, error.offset - 1 - newlinePreError))

  echo json[0 ..< newlinePostError]
  echo indent & "^"
  echo indent & " " & error.msg
  echo json[newlinePostError ..< json.len]

Example usage;

import jsony
from json import JsonNode

type
  thing = tuple
    item: string
    size: string

  jsonContainer = object
    things: seq[thing]
    raptors: bool
    hello: string
    someObjects: JsonNode


var jsonString = """
{
  "things": [
    {"item": "elephant", "size": "the same as a double-decker bus"},
    {"item": "mouse", "size": "smaller than a double-decker bus"},
    {"item": "bus", "size": "also the same as a double-decker bus""},
    {"item": "guitar", "size": "less than a double-decker bus"},
    {"item": "train", "size": "quite a bit more than a double-decker bus"}
  ],
  "raptors": false,
  "hello": "world",
  "someObjects": {
    "dolphin": {
      "legs": 0,
      "eyes": 2,
      "fish": "thanks"
    },
    "ball": {
      "shape": "roundy",
    }
  }
}
"""


var parsedJson: jsonContainer

try:
  parsedJson = jsonString.fromJson(jsonContainer)
except JsonError as e:
  prettyPrintJsonError(jsonString, e)

resulting in an error message like this one;

{
  "things": [
    {"item": "elephant", "size": "the same as a double-decker bus"},
    {"item": "mouse", "size": "smaller than a double-decker bus"},
    {"item": "bus", "size": "also the same as a double-decker bus""},
                                                                  ^
                                                                   Expected } but got " instead.

    {"item": "guitar", "size": "less than a double-decker bus"},
    {"item": "train", "size": "quite a bit more than a double-decker bus"}
  ],
  "raptors": false,
  "hello": "world",
  "someObjects": {
    "dolphin": {
      "legs": 0,
      "eyes": 2,
      "fish": "thanks"
    },
    "ball": {
      "shape": "roundy",
    }
  }
}

Ah, much better! And without any impact on jsony's parse speed. What do you think? Am I overlooking anything important?

[treeform]

Adding offset is responsible but removing it from the error message is bad, not everyone will have a pretty printer.

[finitemonkey]

Yeah, I agree and I almost left the offset in the error message for this PR. Of course, we could just put it back with the only downside of a tiny amount of redundant info.

However, there is another option - what if I improved the pretty-print proc so that it was suitable to include with JSONy? In the comment above, the proc is just a good-enough-for-my-purposes hack, but what if it;

• Properly dealt with Unicode chars.
• Properly dealt with the various line ending possibilities - \n, \r\n or \r
• Trimmed long output to a maximum of ~25 lines and indicated which lines are missing.
• Trimmed long lines to a maximum of ~80 chars and indicated which chars are missing.
• Ensured that the error message always finishes < 80 chars from the left edge.

Then perhaps in the case that we are in a debug build the pretty-print version can be used (without offset tagged onto the end of the error), but in a release build the standard short (and faster to generate) message is used (with offset at the end of the error message)?

[treeform]

My fear is that JSON i deal with is in GBs. I don't want to print full json as it would scroll forever. There is probably a way to have a nice json error though. Some thing that prints only few lines before and after, but if there isn't any lines it prints bytes instead.

[zerodahero]

I would be in favor of including the offset in the error. I'm also typically dealing with large JSON that's compact (no newlines), so I would want my pretty printer to look something more like:

try:
  let data = json.fromJson(Thing)
except JsonError as e:
  echo e.msg
  const context = 75
  echo json[offset - context .. offset + context]
  echo spaces(context) & "^"

Sure, it's definitely pretty blunt, but from a debug standpoint it's typically all I need.

Having the offset in the error would be especially nice so I won't have to parse it out of the message 😄