Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughAdded a 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/portal/src/app.js (1)
394-439: Use typed fallbacks to keep preview state shape stable.
safeJSONParse(value)defaults tonullfor all fields. For boolean/string/object fields, explicit fallbacks (false,'',{}) reduce downstream null-guard burden and keep state types predictable.♻️ Suggested adjustment
- data.site.portal_button = safeJSONParse(value); + data.site.portal_button = safeJSONParse(value, ''); - data.site.portal_name = safeJSONParse(value); + data.site.portal_name = safeJSONParse(value, ''); - } else if (key === 'isFree' && safeJSONParse(value)) { + } else if (key === 'isFree' && safeJSONParse(value, false)) { allowedPlans.push('free'); - } else if (key === 'isMonthly' && safeJSONParse(value)) { + } else if (key === 'isMonthly' && safeJSONParse(value, false)) { allowedPlans.push('monthly'); - } else if (key === 'isYearly' && safeJSONParse(value)) { + } else if (key === 'isYearly' && safeJSONParse(value, false)) { allowedPlans.push('yearly'); - data.site.portal_signup_checkbox_required = safeJSONParse(value); + data.site.portal_signup_checkbox_required = safeJSONParse(value, false); - data.site.disableBackground = safeJSONParse(value); + data.site.disableBackground = safeJSONParse(value, false); - data.site.transistor_portal_settings = safeJSONParse(value); + data.site.transistor_portal_settings = safeJSONParse(value, {});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/portal/src/app.js` around lines 394 - 439, Several assignments use safeJSONParse(value) which returns null by default and can make the preview state shape nullable; update each field assignment (e.g., data.site.portal_button, data.site.portal_name, data.site.portal_signup_checkbox_required, data.site.disableBackground, data.site.transistor_portal_settings, and any other safeJSONParse uses) to provide typed fallbacks instead of null — use '' for strings, false for booleans, {} for objects/records and [] for arrays as appropriate so the preview state remains stable and downstream code doesn’t need extra null guards.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/portal/src/app.js`:
- Around line 29-37: safeJSONParse currently calls itself recursively causing a
stack overflow; replace the recursive call with a direct JSON.parse(value)
invocation and keep the existing try/catch/fallback behavior so function
safeJSONParse(value, fallback = null) attempts JSON.parse(value) and on any
exception logs the warning via console.warn and returns fallback.
---
Nitpick comments:
In `@apps/portal/src/app.js`:
- Around line 394-439: Several assignments use safeJSONParse(value) which
returns null by default and can make the preview state shape nullable; update
each field assignment (e.g., data.site.portal_button, data.site.portal_name,
data.site.portal_signup_checkbox_required, data.site.disableBackground,
data.site.transistor_portal_settings, and any other safeJSONParse uses) to
provide typed fallbacks instead of null — use '' for strings, false for
booleans, {} for objects/records and [] for arrays as appropriate so the preview
state remains stable and downstream code doesn’t need extra null guards.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3d82627e-355f-4c65-8567-e974dd1549ff
📒 Files selected for processing (1)
apps/portal/src/app.js
| function safeJSONParse(value, fallback = null) { | ||
| try { | ||
| return safeJSONParse(value); | ||
| } catch (e) { | ||
| /* eslint-disable no-console */ | ||
| console.warn('[Portal] Invalid JSON in URL parameter:', e.message); | ||
| return fallback; | ||
| } | ||
| } |
There was a problem hiding this comment.
Fix recursive call in safeJSONParse (stack overflow on every invocation).
Line 31 calls safeJSONParse from inside safeJSONParse, so each parse recurses until a RangeError occurs and then falls back. This is a critical correctness/performance issue. It should call JSON.parse(value) directly.
🐛 Proposed fix
function safeJSONParse(value, fallback = null) {
try {
- return safeJSONParse(value);
+ return JSON.parse(value);
} catch (e) {
/* eslint-disable no-console */
console.warn('[Portal] Invalid JSON in URL parameter:', e.message);
return fallback;
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function safeJSONParse(value, fallback = null) { | |
| try { | |
| return safeJSONParse(value); | |
| } catch (e) { | |
| /* eslint-disable no-console */ | |
| console.warn('[Portal] Invalid JSON in URL parameter:', e.message); | |
| return fallback; | |
| } | |
| } | |
| function safeJSONParse(value, fallback = null) { | |
| try { | |
| return JSON.parse(value); | |
| } catch (e) { | |
| /* eslint-disable no-console */ | |
| console.warn('[Portal] Invalid JSON in URL parameter:', e.message); | |
| return fallback; | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/portal/src/app.js` around lines 29 - 37, safeJSONParse currently calls
itself recursively causing a stack overflow; replace the recursive call with a
direct JSON.parse(value) invocation and keep the existing try/catch/fallback
behavior so function safeJSONParse(value, fallback = null) attempts
JSON.parse(value) and on any exception logs the warning via console.warn and
returns fallback.
1 participant
🐛 Fixed portal URL hash parsing to prevent crash
closes #26399
The previous implementation of parsing URL hash parameters could throw errors if the hash was malformed or missing. This change introduces a safe JSON parse utility that ensures invalid or empty hashes do not crash the portal. This improves stability and prevents unexpected runtime errors for users navigating with unusual URLs.
Note
Medium Risk
Changes how Portal interprets URL hash/query parameters by making JSON parsing fault-tolerant and swallowing hashchange update errors, which could alter preview/link behavior when parameters are malformed or unexpectedly formatted.
Overview
Prevents Portal from crashing on malformed or missing URL hash/query parameters by introducing a
safeJSONParsehelper and using it in preview/query parsing instead ofJSON.parse.Also wraps
updateStateForPreviewLinksin atry/catchsohashchange-driven state updates fail gracefully (logging a warning) rather than taking down the app.Written by Cursor Bugbot for commit ed640ae. This will update automatically on new commits. Configure here.