Improve error handling for malformed JSON syntax #13
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
|
Co-authored-by: CNSeniorious000 <[email protected]>
Copilot
AI
changed the title
commit: |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
WalkthroughThe changes improve JSON parsing by enhancing error handling for malformed numbers and invalid tokens, ensuring that syntax errors are no longer silently ignored. The parser now strictly validates number formats, propagates Changes
Sequence Diagram(s)sequenceDiagram
participant TestSuite as Test Suite
participant Parser as JSON Parser
participant Error as MalformedJSON
TestSuite->>Parser: parse(JSON string)
alt Valid JSON
Parser-->>TestSuite: Return parsed object/array
else Invalid number or token
Parser-->>Error: Throw MalformedJSON
Error-->>TestSuite: Error is caught and asserted
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Assessment against linked issues
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tests/issue12.test.js (1)
15-19: Consider adding more invalid token test casesThe tests correctly verify that invalid tokens throw errors. Consider adding more edge cases for completeness.
test("issue #12 - invalid tokens should throw error instead of returning empty", () => { // Should throw error instead of returning [] expect(() => parse("[abc")).toThrow(MalformedJSON); expect(() => parse("[invalid")).toThrow(MalformedJSON); + expect(() => parse("{abc}")).toThrow(MalformedJSON); + expect(() => parse("[#invalid]")).toThrow(MalformedJSON); + expect(() => parse("[.abc]")).toThrow(MalformedJSON); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/index.ts(4 hunks)tests/issue12.test.js(1 hunks)
🔇 Additional comments (8)
src/index.ts (5)
117-131: Good error propagation patternThe explicit re-throwing of
MalformedJSONerrors ensures they bubble up correctly without being masked by the partial parsing logic.
141-149: Improved whitespace handling in arraysThe addition of
skipBlank()calls ensures proper handling of whitespace in arrays, fixing the issue where empty arrays with spaces would cause parsing failures.
153-156: Consistent error propagation in array parsingThe error handling matches the pattern used in
parseObj, ensuringMalformedJSONerrors bubble up correctly.
191-203: Robust partial number parsingThe improved error handling for partial numbers is more explicit and maintains backward compatibility while properly reporting malformed JSON.
68-75: Fix variable shadowing in error messageThe implementation correctly validates number tokens before parsing. However, the error message on line 74 uses
charwhich shadows the const declaration.- throwMalformedError(`Unexpected token '${char}'`); + throwMalformedError(`Unexpected token '${jsonString[index]}'`);Likely an incorrect or invalid review comment.
tests/issue12.test.js (3)
4-13: Comprehensive test coverage for invalid number formatsThe tests effectively cover the main issue from #12, verifying that numbers starting with dots throw
MalformedJSONerrors in various contexts.
21-34: Well-structured test for whitespace handlingThe test effectively verifies that empty arrays with spaces are handled correctly and don't interrupt the parsing of subsequent elements.
36-43: Good regression test coverageThe tests ensure that valid JSON continues to parse correctly after the error handling improvements, preventing regressions.
CNSeniorious000
changed the title
Fixes #12 by properly distinguishing between malformed JSON (invalid syntax) and partial JSON (incomplete but valid syntax).
Problem
The parser was silently swallowing invalid JSON syntax instead of throwing errors:
.0516156161551515would be silently ignored, causing the rest of valid JSON to be discarded[abcwould return[]instead of throwing an error[ ]would cause parsing to terminate early, discarding subsequent valid JSONExample
Before this fix:
After this fix:
Solution
Enhanced token validation - Added proper validation before attempting to parse numbers, rejecting invalid tokens like
.123andabcImproved array parsing - Fixed whitespace handling in arrays by adding
skipBlank()calls at appropriate pointsBetter error propagation - Modified
parseArr()andparseObj()to letMalformedJSONerrors bubble up while still allowingPartialJSONerrors to be handled gracefully for incomplete but valid JSONBackward Compatibility
✅ All existing functionality is preserved:
Allowflags continue to control partial parsing behavior as expectedThis change only affects clearly invalid JSON syntax that should never have been accepted in the first place.
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.
Summary by CodeRabbit
Bug Fixes
Tests