Improve json decode error messages

[dgud]

When printing errors include the position that the error occured at if included in the error.

[github-actions]

CT Test Results

    2 files     97 suites   1h 8m 4s ⏱️
2 192 tests 2 145 ✅ 47 💤 0 ❌
2 559 runs  2 510 ✅ 49 💤 0 ❌

Results for commit 1259bbb.

♻️ This comment has been updated with latest results.

To speed up review, make sure that you have read Contributing to Erlang/OTP and that all checks pass.

See the TESTING and DEVELOPMENT HowTo guides for details about how to run test locally.

Artifacts

• Complete CT logs (Download Logs)
• HTML Documentation (Download HTML Docs)
• Windows Installer

// Erlang/OTP Github Action Bot

[garazdawi]

Is the position a character? or a byte?

[dgud]

What is a character in unicode?
I wrote position for a reason :-)
@michalmuskala

[garazdawi]

erlang:char/0 is a character in Erlang :)

[michalmuskala]

It's a byte offset

[bjorng]

Test case?

[dgud]

Test case?

How do I write one, can point me to any examples?

[bjorng]

How do I write one, can point me to any examples?

otp/lib/stdlib/test/maps_SUITE.erl

Lines 943 to 1065 in d24c732

	error_info(_Config) ->
	BadIterator = [-1|#{}],
	BadIterator2 = {x, y, z},
	GoodIterator = maps:iterator(#{}),
	BadOrder = fun(_) -> true end,
	GoodOrder = fun(A, B) -> A =< B end,
	L = [
	{filter, [fun(_, _) -> true end, abc]},
	{filter, [fun(_, _) -> true end, BadIterator]},
	{filter, [fun(_, _) -> true end, BadIterator2]},
	{filter, [bad_fun, BadIterator],[{1,".*"},{2,".*"}]},
	{filter, [bad_fun, BadIterator2],[{1,".*"},{2,".*"}]},
	{filter, [bad_fun, GoodIterator]},
	{filtermap, [fun(_, _) -> true end, abc]},
	{filtermap, [fun(_, _) -> true end, BadIterator]},
	{filtermap, [fun(_, _) -> true end, BadIterator2]},
	{filtermap, [fun(_) -> true end, GoodIterator]},
	{filtermap, [fun(_) -> ok end, #{}]},
	{find, [key, no_map]},
	{fold, [fun(_, _, _) -> true end, init, abc]},
	{fold, [fun(_, _, _) -> true end, init, BadIterator]},
	{fold, [fun(_, _, _) -> true end, init, BadIterator2]},
	{fold, [fun(_) -> true end, init, GoodIterator]},
	{fold, [fun(_) -> ok end, init, #{}]},
	{foreach, [fun(_, _) -> ok end, no_map]},
	{foreach, [fun(_, _) -> ok end, BadIterator]},
	{foreach, [fun(_, _) -> ok end, BadIterator2]},
	{foreach, [fun(_) -> ok end, GoodIterator]},
	{foreach, [fun(_) -> ok end, #{}]},
	{from_keys, [#{a => b}, whatever]},
	{from_keys, [[a|b], whatever]},
	{from_list, [#{a => b}]},
	{from_list, [[a|b]]},
	{get, [key, #{}]},
	{get, [key, {no,map}]},
	{get, [key, {no,map}, default]},
	{groups_from_list, [not_a_fun, []]},
	{groups_from_list, [fun hd/1, not_a_list]},
	{groups_from_list, [not_a_fun, fun(_) -> ok end, []]},
	{groups_from_list, [fun(_) -> ok end, not_a_fun, []]},
	{groups_from_list, [fun(_) -> ok end, fun(_) -> ok end, not_a_list]},
	{intersect, [#{a => b}, y]},
	{intersect, [x, #{a => b}]},
	{intersect, [x, y],[{1,".*"},{2,".*"}]},
	{intersect_with, [fun(_, _, _) -> ok end, #{a => b}, y]},
	{intersect_with, [fun(_, _, _) -> ok end, x, #{a => b}]},
	{intersect_with, [fun(_, _, _) -> ok end, x, y],[{2,".*"},{3,".*"}]},
	{intersect_with, [fun(_, _) -> ok end, #{}, #{}]},
	{is_iterator_valid, [GoodIterator], [no_fail]},
	{is_iterator_valid, [BadIterator], [no_fail]},
	{is_iterator_valid, [BadIterator2], [no_fail]},
	{is_key,[key, no_map]},
	{iterator,[{no,map}]},
	{iterator, [{no,map}, undefined], [{1, ".*"}]},
	{iterator, [{no,map}, ordered], [{1, ".*"}]},
	{iterator, [{no,map}, reversed], [{1, ".*"}]},
	{iterator, [{no,map}, GoodOrder], [{1, ".*"}]},
	{iterator, [#{a => b}, BadOrder], [{2, ".*"}]},
	{iterator, [{no,map}, BadOrder], [{1, ".*"}, {2, ".*"}]},
	{keys, [{no,map}]},
	{map, [fun(_, _) -> true end, abc]},
	{map, [fun(_, _) -> true end, BadIterator]},
	{map, [fun(_, _) -> true end, BadIterator2]},
	{map, [fun(_) -> true end, GoodIterator]},
	{map, [fun(_) -> ok end, #{}]},
	{merge,[#{a => b}, {a,b}]},
	{merge,[{x,y}, #{a => b}]},
	{merge,[{x,y}, {a,b}],[{1,".*"},{2,".*"}]},
	{merge_with, [fun(_, _) -> ok end, #{}, #{}]},
	{merge_with, [a, b, c],[{1,".*"},{2,".*"},{3,".*"}]},
	{next,[no_iterator]},
	{next,[BadIterator]},
	{put, [key, value, {no,map}]},
	{remove,[key, {no,map}]},
	{size, [no_map]},
	{take, [key, no_map]},
	{to_list,[xyz]},
	{to_list,[BadIterator]},
	{to_list,[BadIterator2]},
	{update,[key, value, no_map]},
	{update_with, [key, fun(_) -> ok end, {no,map}]},
	{update_with, [key, fun() -> ok end, #{}]},
	{update_with, [key, fun(_) -> ok end, init, {no,map}]},
	{update_with, [key, fun() -> ok end, init, #{}]},
	{values,[no_map]},
	{with, [not_list, #{}]},
	{with, [[1,2,3], {no,map}]},
	{without, [not_list, #{}]},
	{without, [[1,2,3], {no,map}]}
	],
	error_info_lib:test_error_info(maps, L).

[bjorng]

"Occurred" is misspelled.

Instead of simply fixing the misspelling, I suggest that you generate a clearer message:

Suggested change

	format_json_error(_F, _As, #{position := Position}) ->
	[{general, io_lib:format("Occured at position ~w", [Position])}];
	format_json_error({invalid_byte, Byte}, #{position := Position}) ->
	S = io_lib:format("invalid byte ~w at position ~w", [Byte, Position]),
	[{general, S}];

(You will have to update the call to format_json_error, too.)

This will result in some redundancy, but overall I think it's clearer:

1> json:decode(~'["valid string", not_valid]').
** exception error: {invalid_byte,111}
     in function  json:invalid_byte/2 (json.erl, line 541)
        *** invalid byte 111 at position 18
     in call from json:decode/1 (json.erl, line 879)

[dgud]

Changed it to include the character as well, might help if it is an ASCII character.

1> json:decode(<<"["valid string", #not_valid">>).
** exception error: {invalid_byte,35}
in function json:invalid_byte/2 (json.erl, line 541)
*** Invalid byte 35 '#' at byte position 17
in call from json:decode/1 (json.erl, line 879)
2>

[bjorng]

What if it is a control character?

[dgud]

Looks ok to me

11> json:decode(<<"[\"valid string\"", 8>>).
** exception error: {invalid_byte,8}
     in function  json:invalid_byte/2 (json.erl, line 541)
        *** Invalid byte 8 '^(' at byte position 15
     in call from json:decode/1 (json.erl, line 879)

[bjorng]

Some of the control characters looks fishy to me:

1> json:decode(<<"[\"valid string\"", 9>>).
** exception error: unexpected_end
     in function  json:decode/1 (json.erl, line 889)
2> json:decode(<<"[\"valid string\"", 10>>).
** exception error: unexpected_end
     in function  json:decode/1 (json.erl, line 889)
3> json:decode(<<"[\"valid string\"", 11>>).
** exception error: {invalid_byte,11}
     in function  json:invalid_byte/2 (json.erl, line 541)
        *** Invalid byte 11 '^+' at byte position 15
     in call from json:decode/1 (json.erl, line 879)
4> json:decode(<<"[\"valid string\"", 12>>).
** exception error: {invalid_byte,12}
     in function  json:invalid_byte/2 (json.erl, line 541)
        *** Invalid byte 12 '^,' at byte position 15
     in call from json:decode/1 (json.erl, line 879)
5> json:decode(<<"[\"valid string\"", 27>>).
** exception error: {invalid_byte,27}
     in function  json:invalid_byte/2 (json.erl, line 541)
        *** Invalid byte 27 ' at byte position 15
     in call from json:decode/1 (json.erl, line 879)

[michalmuskala]

In Jason we print the byte in hex notation, and as a string if it happens to be a printable character, see

https://github.com/michalmuskala/jason/blob/984bc078eb4b2084104751c7f1c5290b8338e06b/lib/decoder.ex#L12-L23

[dgud]

Good point and the byte is already included as a decimal integer in the original error reason.