Improve JSON decode error messages with path context #4302
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This commit improves error messages when JSON decoding fails by adding context about where in the JSON structure the error occurred. Changes: - Added pathContext field to Decoder to track the current location - Added path management helper methods (pushPath, popPath, getPathString) - Added *WithContext versions of jsonObject methods that use path context - Updated key decode methods to track and use path context For example, instead of: missing property: type Users now see: missing property: type (at array[0]) missing property: type (at field:myField.type) Closes #4209
Follow-up to address code review feedback from @turbolent. Changes: - Introduced pathElement interface with Append(io.Writer) method - Created specific path element types: - indexPathElement: for array indices - fieldPathElement: for struct/resource/event field names - staticPathElement: for static strings like 'type' - Updated getPathString() to use strings.Builder for efficient concatenation - Changed pathContext from []string to []pathElement This optimization avoids string formatting and allocation during the happy path (successful decoding), only building the path string when an error occurs. Addresses: #4288 (comment)
Added comprehensive tests to verify that path context is correctly included in error messages when JSON decoding fails. Test cases cover: - Missing property at root level - Missing property in array elements - Missing property in nested array elements - Missing property in struct fields - Missing property in type definitions - Missing property in complex nested structures (array[0].field:nested) All tests verify that the error message contains both the property name and the path context showing where in the JSON structure the error occurred.
Comprehensive refactoring to consistently use GetWithContext, GetStringWithContext, GetSliceWithContext, GetBoolWithContext, and GetValueWithContext throughout all decode methods. This ensures path context is available for all decoding errors, not just in a few select locations. Now any missing property error will include context about where in the JSON structure the error occurred. Updated methods: - decodeKeyValuePair - decodeComposite - decodeInclusiveRange - decodePath - decodeFunction - decodeTypeParameter - decodeParameter - decodeFieldType - decodeAuthorization - decodeNominalType - decodeType - decodeTypeValue - decodeCapability Addresses: #4288 (review)
- Add path tracking system using pathElement interface - Implement indexPathElement, fieldPathElement, staticPathElement - Refactor all decode methods to use *WithContext helpers - Remove non-context Get* helper methods - Add comprehensive error context tests Closes #4209
6 tasks
Benchstat comparison
Results
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
6 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #4209
Description
Finish and improve the work started in #4288.
In errors that occur during decoding JSON for a particular property or index, include the path in the error.
This requires refactoring the decoding so it is performed while the path is up-to-date. Currently the properties are get, then decoded "out-of-context". Add a new
getfunction that uses continuation-passing style to decode a property. Also update the path for all loops.Best viewed by ignoring whitespace changes: https://github.com/onflow/cadence/pull/4302/files?w=1
masterbranchFiles changedin the Github PR explorer