-
Notifications
You must be signed in to change notification settings - Fork 286
[ACTVTNS-387] JSON Decode Error #3368
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
This PR makes changes to mapping-kit. Please ensure that the changes are reflected in the mapping-kit go library as well and link the PR in description. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3368 +/- ##
==========================================
+ Coverage 79.99% 80.34% +0.34%
==========================================
Files 1207 1260 +53
Lines 22315 25168 +2853
Branches 4398 5149 +751
==========================================
+ Hits 17851 20220 +2369
- Misses 3688 4166 +478
- Partials 776 782 +6 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes a JSON decode error in the mapping-kit where non-JSON primitive string values (like plain text, numbers, or booleans as strings) would cause JSON.parse() to throw an error, resulting in a "Could not load page" error for users.
Key Changes:
- Added try-catch error handling to the
@jsondirective's decode mode to gracefully handle invalid JSON strings - Invalid JSON strings now return the original value instead of throwing an error
- Added unit test to verify the fix works correctly
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/core/src/mapping-kit/index.ts | Added try-catch block around JSON.parse() to handle invalid JSON gracefully and return original value on parse failure |
| packages/core/src/mapping-kit/tests/index.iso.test.ts | Added test case to verify that invalid JSON strings are returned as-is without throwing errors |
| } else if (opts.mode === 'decode') { | ||
| if (typeof value === 'string') { | ||
| return JSON.parse(value) | ||
| //Placing this in a try-catch to safely handle any primiative values that are put in the JSON.parse |
Copilot
AI
Oct 24, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Corrected spelling of 'primiative' to 'primitive'.
| //Placing this in a try-catch to safely handle any primiative values that are put in the JSON.parse | |
| //Placing this in a try-catch to safely handle any primitive values that are put in the JSON.parse |
| } else if (opts.mode === 'decode') { | ||
| if (typeof value === 'string') { | ||
| return JSON.parse(value) | ||
| //Placing this in a try-catch to safely handle any primiative values that are put in the JSON.parse |
Copilot
AI
Oct 24, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment should follow standard formatting with a space after the double slashes: // Placing this in a try-catch... Additionally, consider making the comment more precise about what primitive values are being handled (e.g., 'non-JSON strings').
| //Placing this in a try-catch to safely handle any primiative values that are put in the JSON.parse | |
| // Placing this in a try-catch to safely handle non-JSON strings or primitive values that may cause JSON.parse to throw |
Co-authored-by: Marín Alcaraz <[email protected]>
Problem
Customers encountered an error when using the JSON decode function in destination mappings with primitive string values (e.g., plain text, numbers, or booleans as strings) rather than valid JSON strings.
When this occurred, users would see a "Could not load page" error because JSON.parse() would throw an error when attempting to parse non-JSON strings like plain text values.
Solution
Added error handling to the @JSON directive's decode mode in the mapping-kit to gracefully handle invalid JSON strings, similar to how the @case directive handles non-string inputs.
Changes:
Wrapped JSON.parse(value) in a try-catch block in packages/core/src/mapping-kit/index.ts
When JSON parsing fails, the directive now returns the original string value instead of throwing an error
This allows primitive string values to pass through unchanged rather than breaking the mapping
Example:
Before: Would throw error
{ '@JSON': { mode: 'decode', value: 'Hello World' } }
After: Returns original string
{ '@JSON': { mode: 'decode', value: 'Hello World' } } // Returns: 'Hello World'
Testing
Added a comprehensive unit test decode_invalid_json_returns_original_string in index.iso.test.ts that verifies:
Non-JSON strings are returned as-is without throwing errors
The decode functionality still works correctly for valid JSON strings
The error handling doesn't break existing functionality
This change ensures backward compatibility while preventing user-facing errors when non-JSON values are accidentally passed to the decode function.
More information: https://twilio-engineering.atlassian.net/browse/ACTVTNS-387
Testing
Screen.Recording.2025-10-24.at.5.28.03.PM.mov
](url)
Include any additional information about the testing you have completed to
ensure your changes behave as expected. For a speedy review, please check
any of the tasks you completed below during your testing.