Issue 6551 in v8: JSON.parse errors can be user-friendly for short strings

[michalwa… via monorail]

Status: Untriaged
Owner: ----

New issue 6551 by michalwa...@gmail.com: JSON.parse errors can be user-friendly for short strings
https://bugs.chromium.org/p/v8/issues/detail?id=6551

Version: All
OS: All
Architecture: All

What steps will reproduce the problem?

Consider error messages for these codes:
JSON.parse(NaN)
JSON.parse(undefined); // very common bug
JSON.parse(Infinity);
JSON.parse('Bad string');
JSON.parse({});


What is the expected output?

Better error message for failing to parse short (<30 chars?) strings.

So:
SyntaxError: undefined is not a valid JSON;
SyntaxError: "Bad string" is not a valid JSON;
SyntaxError: "[object Object]" is not a valid JSON; // Or special path for logging objects with unparsable .toString()


What do you see instead?

Syntax Error: Unexpected token o in JSON at position 1; // What?
SyntaxError: Unexpected token N in JSON at position 0; // What?
SyntaxError: Unexpected token u in JSON at position 0; // Oh, it's undefined again






--
You received this message because:
1. The project was configured to send all issue notifications to this address

You may adjust your notification preferences at:
https://bugs.chromium.org/hosting/settings

[math… via monorail]

Issue 6551: JSON.parse errors can be user-friendly for short strings
https://bugs.chromium.org/p/v8/issues/detail?id=6551

This issue is now blocking issue 6513.
See https://bugs.chromium.org/p/v8/issues/detail?id=6513

[math… via monorail]

Updates:
Owner: gsat...@chromium.org
Status: Assigned

Comment #2 on issue 6551 by mat...@chromium.org: JSON.parse errors can be user-friendly for short strings
https://bugs.chromium.org/p/v8/issues/detail?id=6551#c2

(No comment was entered for this change.)

[issac… via monorail]

Updates:
Owner: issac...@microsoft.com

Comment #5 on issue 6551 by issac...@microsoft.com: JSON.parse errors can be user-friendly for short strings
https://bugs.chromium.org/p/v8/issues/detail?id=6551#c5

[Git Watcher via monorail]

Comment #6 on issue 6551 by Git Watcher: JSON.parse errors can be user-friendly for short strings
https://bugs.chromium.org/p/v8/issues/detail?id=6551#c6

The following revision refers to this bug:
https://chromium.googlesource.com/chromium/src/+/30ca8f2b2f5b79d5f1c46623ae2c4c565b3e299c

commit 30ca8f2b2f5b79d5f1c46623ae2c4c565b3e299c
Author: Issack John <issac...@microsoft.com>
Date: Thu May 12 21:30:14 2022

Allow fast/js/JSON-parse.html to fail

The error messages are being modified for JSON.parse() errors in
https://chromium-review.googlesource.com/c/v8/v8/+/3513684
which are causing these blink tests to fail.

Bug: v8:6551
Change-Id: I747cd1b43c7d20bddb086b4dc5a8b488d3673fc1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3518659
Reviewed-by: Luis Fernando Pardo Sixtos <lpardo...@microsoft.com>
Commit-Queue: Issack John <issac...@microsoft.com>
Cr-Commit-Position: refs/heads/main@{#1002853}

[modify] https://crrev.com/30ca8f2b2f5b79d5f1c46623ae2c4c565b3e299c/third_party/blink/web_tests/TestExpectations

[issac… via monorail]

Updates:
Labels: msft-consider

Comment #7 on issue 6551 by issac...@microsoft.com: JSON.parse errors can be user-friendly for short strings
https://bugs.chromium.org/p/v8/issues/detail?id=6551#c7

(No comment was entered for this change.)

[Git Watcher via monorail]

Comment #8 on issue 6551 by Git Watcher: JSON.parse errors can be user-friendly for short strings
https://bugs.chromium.org/p/v8/issues/detail?id=6551#c8

The following revision refers to this bug:

https://chromium.googlesource.com/v8/v8/+/718f743750135b3a3e643655a0406a90bf4bf010

commit 718f743750135b3a3e643655a0406a90bf4bf010
Author: Issack John <issac...@microsoft.com>
Date: Mon May 16 15:53:06 2022

JSON.parse errors made user-friendly

Part of the improve error messages initiative.

Based on a resource of JSON.parse() errors found at
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Errors/JSON_bad_parse

Previously JSON.parse(NaN) would output:

SyntaxError: Unexpected token N in JSON at position 0

Now the output is:
SyntaxError: "NaN" is not valid JSON

Previously JSON.parse("{a:1}") would output:
SyntaxError: Unexpected token a in JSON at position 1
Now the output is:
SyntaxError: Expected property name or '}' in JSON at position 1

Bug: v8:6551
Change-Id: Ic9fad1fdbd295e1302805b81e6603fc526121960
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3513684
Reviewed-by: Toon Verwaest <verw...@chromium.org>
Commit-Queue: Issack John <issac...@microsoft.com>
Cr-Commit-Position: refs/heads/main@{#80567}

[modify] https://crrev.com/718f743750135b3a3e643655a0406a90bf4bf010/test/mjsunit/messages.js
[modify] https://crrev.com/718f743750135b3a3e643655a0406a90bf4bf010/test/message/fail/modules-import-assertions-fail-2.out
[modify] https://crrev.com/718f743750135b3a3e643655a0406a90bf4bf010/src/json/json-parser.cc
[modify] https://crrev.com/718f743750135b3a3e643655a0406a90bf4bf010/src/common/message-template.h
[modify] https://crrev.com/718f743750135b3a3e643655a0406a90bf4bf010/test/inspector/runtime/regress-1078205-expected.txt
[modify] https://crrev.com/718f743750135b3a3e643655a0406a90bf4bf010/test/mjsunit/json-errors.js
[modify] https://crrev.com/718f743750135b3a3e643655a0406a90bf4bf010/src/json/json-parser.h

[Git Watcher via monorail]

Comment #9 on issue 6551 by Git Watcher: JSON.parse errors can be user-friendly for short strings
https://bugs.chromium.org/p/v8/issues/detail?id=6551#c9

The following revision refers to this bug:

https://chromium.googlesource.com/v8/v8/+/a679edbb6eac031b32412a2e3a04131ac617ecae

commit a679edbb6eac031b32412a2e3a04131ac617ecae
Author: Issack John <issac...@microsoft.com>
Date: Wed May 18 16:12:47 2022

JSON.parse errors made user-friendly part 2

Part of the improve error messages initiative.

Based on a resource of JSON.parse() errors found at
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Errors/JSON_bad_parse

added support for:
- 'Bad control character in string literal'
- 'Bad Unicode escape'

Previously JSON.parse('"a\bz"') would output:
SyntaxError: Unexpected token in JSON at position 2
Now the output is:
SyntaxError: Bad control character in string literal in
JSON at position 2

Previously JSON.parse("[\"\\t\\u") would output:
SyntaxError: Unexpected end of JSON input
Now the output is:
SyntaxError: Bad Unicode escape in JSON at position 6

Bug: v8:6551
Change-Id: I3ba5450c41b8a388643a15bc58e4e3fc75855d13
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3652254

Reviewed-by: Toon Verwaest <verw...@chromium.org>
Commit-Queue: Issack John <issac...@microsoft.com>

Cr-Commit-Position: refs/heads/main@{#80642}

[modify] https://crrev.com/a679edbb6eac031b32412a2e3a04131ac617ecae/test/mjsunit/messages.js
[modify] https://crrev.com/a679edbb6eac031b32412a2e3a04131ac617ecae/src/json/json-parser.cc
[modify] https://crrev.com/a679edbb6eac031b32412a2e3a04131ac617ecae/src/common/message-template.h
[modify] https://crrev.com/a679edbb6eac031b32412a2e3a04131ac617ecae/test/mjsunit/json-errors.js

[issac… via monorail]

Updates:
Status: Started

Comment #10 on issue 6551 by issac...@microsoft.com: JSON.parse errors can be user-friendly for short strings
https://bugs.chromium.org/p/v8/issues/detail?id=6551#c10

(No comment was entered for this change.)

[Git Watcher via monorail]

Comment #11 on issue 6551 by Git Watcher: JSON.parse errors can be user-friendly for short strings
https://bugs.chromium.org/p/v8/issues/detail?id=6551#c11

The following revision refers to this bug:

https://chromium.googlesource.com/chromium/src/+/4ea31dcee78a3f999d63eedeb1780e794b632959

commit 4ea31dcee78a3f999d63eedeb1780e794b632959
Author: Issack John <issac...@microsoft.com>
Date: Fri May 20 17:37:51 2022

Rebaseline fast/js/JSON-parse.html

The error messages are being modified for JSON.parse() in

https://chromium-review.googlesource.com/c/v8/v8/+/3513684
which are causing these blink tests to fail.

Removed the [ Failure Pass ] entry from the test expectations file.

Bug: v8:6551
Change-Id: I626a220d1099ddf84331b9f4b7e31d7f3e7e1a14
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3650941

Reviewed-by: Luis Fernando Pardo Sixtos <lpardo...@microsoft.com>

Commit-Queue: Issack John <issac...@microsoft.com>
Cr-Commit-Position: refs/heads/main@{#1005862}

[modify] https://crrev.com/4ea31dcee78a3f999d63eedeb1780e794b632959/third_party/blink/web_tests/TestExpectations
[modify] https://crrev.com/4ea31dcee78a3f999d63eedeb1780e794b632959/third_party/blink/web_tests/platform/generic/fast/js/JSON-parse-expected.txt

[issac… via monorail]

Updates:
Status: Fixed

Comment #12 on issue 6551 by issac...@microsoft.com: JSON.parse errors can be user-friendly for short strings
https://bugs.chromium.org/p/v8/issues/detail?id=6551#c12

(No comment was entered for this change.)