Fix #143

[travisbrown]

This is a funny one. The problem is that the new error message stuff from #129 calls at(i, j) with out-of-bounds arguments on AsyncParser, which throws an AsyncException instead of a ParseException, which means the absorb call isn't treated as a failure.

My feeling is that we're going to keep seeing problems introduced by #129 for a while, starting with #142, which I'm hoping to get to next.

r? @BenFradet @non

[BenFradet]

Nice, thanks for digging into it!

[travisbrown]

Note that this means that Parser.parseFromString("}x")(NullFacade) fails with the following:

ParseException: expected json value got '}...' (line 1, column 1)

…instead of the current:

ParseException: expected json value got '}x' (line 1, column 1)

…which seems reasonable to me, but we could alternatively fix this by making sure die doesn't leak AsyncExceptions.

[rossabaker]

Some more things that return Right on absorb even though they're already bad:

• fälse
• nüll
• trüe
• 0x
• 0.x
• 0ex
• [1; 2]
• {"a"; 1}
• {1: 2}
• I'm pretty sure a Unicode one, but I couldn't make it die in the right place

This fix works, but maybe it's worth exploring that alternative idea with the leaking AsyncExceptions to handle more of these.

[travisbrown]

@rossabaker Nice. And agreed. I'm happy to take a stab at it but it might be a couple of days, so if someone else wants to deal with it sooner I'm happy to review.

[travisbrown]

Just pushed a fix that makes sure die doesn't throw AsyncException.

[rossabaker]

I like it. Add a @tailrec if both you and scalac find it agreeable, but 👍 either way.

[rossabaker]

j-i is at most ErrorContext (which is 6), right? Just wondering about catching and throwing absurd numbers of exceptions as it walks back, but I won't lose sleep over up to 6. And I think it's @tailrec, though I didn't ask scalac.

[travisbrown]

In theory you could pass some ridiculously large context to die, but it's protected, so I'm not too worried about that. Also except in the (kind of weird) AsyncParser usage the at call in safeAt will never throw exceptions at all, since atEof has been checked.

(It is @tailrec, by the way, so I went ahead and added it.)