[FIX] Improve platform key error with actionable link to settings#1878
Merged
chandrasekharan-zipstack merged 2 commits intomainfrom Mar 30, 2026
Merged
[FIX] Improve platform key error with actionable link to settings#1878chandrasekharan-zipstack merged 2 commits intomainfrom
chandrasekharan-zipstack merged 2 commits intomainfrom
Conversation
…e link - Improve ActiveKeyNotFound error to include markdown link to Platform Keys settings - Remove dead MissingEnvException class (zero usages) - Add generic enrichMarkdownLinks in useExceptionHandler to resolve relative markdown links with org prefix via buildAlert helper - Support internal links in CustomMarkdown via React Router for SPA navigation Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
Summary by CodeRabbit
WalkthroughUpdates exception messaging and removes a duplicate backend exception; adds org-aware markdown link rewriting in the frontend exception handler and renders internal markdown links as in-app router links instead of external anchors. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Contributor
Greptile SummaryThis PR improves the Changes:
The changes are well-scoped and the enrichment is a no-op for messages that contain no markdown links, so existing error paths are unaffected. Confidence Score: 5/5Safe to merge — all changes are additive, the enrichment function is a no-op on existing messages, and external link behaviour is unchanged. No P0 or P1 issues found. The one previously-flagged P1 ($-pattern expansion) was resolved via a function callback replacement. The remaining open item (RouterLink visual consistency with Ant Design) is a pre-existing P2 already noted in the prior review thread and does not affect correctness or navigation behaviour. All error return paths are correctly covered by the new buildAlert helper. No files require special attention. Important Files Changed
Sequence DiagramsequenceDiagram
participant BE as Backend
participant EH as useExceptionHandler
participant EML as enrichMarkdownLinks
participant CM as CustomMarkdown
participant RR as React Router
BE->>EH: err.response.data.detail contains markdown link
EH->>EML: enrichMarkdownLinks(detail)
EML->>EML: replace [text](/path) with [text](/{orgName}/path)
EML-->>EH: enriched markdown string
EH-->>CM: buildAlert passes enriched content
CM->>CM: parse link token, url.startsWith("/") = true
CM->>RR: RouterLink to orgName-prefixed path
RR-->>CM: SPA navigation without full page reload
Reviews (2): Last reviewed commit: "[FIX] Use replacer function in enrichMar..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
frontend/src/hooks/useExceptionHandler.jsx (1)
19-22: Consider usingreplaceAll()for clarity.While
replace()with a global regex flag already replaces all matches, usingreplaceAll()makes the intent more explicit and aligns with modern JavaScript conventions.♻️ Proposed fix
- return message.replace( + return message.replaceAll( /\[([^\]]+)\]\(\/((?!\/)[^)]+)\)/g, `[$1](/${orgName}/$2)`, );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/hooks/useExceptionHandler.jsx` around lines 19 - 22, Replace the string.replace call with replaceAll to make the intent explicit: in the useExceptionHandler hook, change the return that calls message.replace(/\[([^\]]+)\]\(\/((?!\/)[^)]+)\)/g, `[$1](/${orgName}/$2)`) to use message.replaceAll(...) with the same regex (keep the global flag) and replacement template; also verify the runtime/browser targets support String.prototype.replaceAll or add a polyfill if necessary.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@frontend/src/hooks/useExceptionHandler.jsx`:
- Around line 19-22: Replace the string.replace call with replaceAll to make the
intent explicit: in the useExceptionHandler hook, change the return that calls
message.replace(/\[([^\]]+)\]\(\/((?!\/)[^)]+)\)/g, `[$1](/${orgName}/$2)`) to
use message.replaceAll(...) with the same regex (keep the global flag) and
replacement template; also verify the runtime/browser targets support
String.prototype.replaceAll or add a polyfill if necessary.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: fbdc78bf-ea8b-4a30-8f66-6352755e1d45
📒 Files selected for processing (4)
backend/platform_settings_v2/exceptions.pybackend/workflow_manager/workflow_v2/exceptions.pyfrontend/src/components/helpers/custom-markdown/CustomMarkdown.jsxfrontend/src/hooks/useExceptionHandler.jsx
💤 Files with no reviewable changes (1)
- backend/workflow_manager/workflow_v2/exceptions.py
chandrasekharan-zipstack
requested review from
kirtimanmishrazipstack and
pk-zipstack
… expansion Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
Frontend Lint Report (Biome)✅ All checks passed! No linting or formatting issues found. |
Contributor
Author
|
Addressing Greptile review comments: 1. 2. |
Contributor
Test ResultsSummary
Runner Tests - Full Report
SDK1 Tests - Full Report
|
|
pk-zipstack
approved these changes
Mar 30, 2026
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
frontend/src/hooks/useExceptionHandler.jsx (1)
8-8: Use a selector for Zustand subscription to prevent unnecessary rerenders when unrelated state changes.On Line 8, the hook subscribes to the entire store but only uses
sessionDetails. WhenisLogoutLoadingchanges elsewhere, this hook will unnecessarily rerender. Use a selector to subscribe only to the specific state needed.- const { sessionDetails } = useSessionStore(); + const sessionDetails = useSessionStore((state) => state.sessionDetails);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/hooks/useExceptionHandler.jsx` at line 8, The hook currently subscribes to the entire Zustand store via useSessionStore() but only uses sessionDetails, causing unnecessary rerenders when other keys (e.g., isLogoutLoading) change; update the subscription to select only sessionDetails by replacing the current destructuring with a selector call like useSessionStore(state => state.sessionDetails) in useExceptionHandler (and remove unused destructured values) so the component only re-renders when sessionDetails changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/src/hooks/useExceptionHandler.jsx`:
- Around line 19-22: The markdown-link replacer in useExceptionHandler.jsx
always prefixes `/${orgName}/` causing double-org segments; update the replacer
inside the function where the regex is used to check whether the captured path
already starts with `${orgName}/` (or `/\${orgName}/`) and only prepend
`/${orgName}/` when it does not; reference the existing regex and the replacer
callback (the arrow function using parameters `(_, text, path) => ...`) and use
a conditional inside that callback to avoid double-prefixing.
---
Nitpick comments:
In `@frontend/src/hooks/useExceptionHandler.jsx`:
- Line 8: The hook currently subscribes to the entire Zustand store via
useSessionStore() but only uses sessionDetails, causing unnecessary rerenders
when other keys (e.g., isLogoutLoading) change; update the subscription to
select only sessionDetails by replacing the current destructuring with a
selector call like useSessionStore(state => state.sessionDetails) in
useExceptionHandler (and remove unused destructured values) so the component
only re-renders when sessionDetails changes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2465cb87-b1e5-45ef-8ed7-a1345744a91e
📒 Files selected for processing (1)
frontend/src/hooks/useExceptionHandler.jsx
Contributor
Author
|
False positive — |
kirtimanmishrazipstack
approved these changes
Mar 30, 2026
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
What
ActiveKeyNotFounderror message with an actionable description and a clickable link to the Platform Keys settings pageMissingEnvExceptionclassWhy
MissingEnvExceptioninworkflow_managerwas a duplicate ofActiveKeyNotFoundwith zero imports/usagesHow
ActiveKeyNotFound.default_detailto include a markdown link ([Platform Keys](/settings/platform)) pointing to the settings pageuseExceptionHandler): Added genericenrichMarkdownLinks()that resolves relative markdown links ([text](/path)) by prepending the org name ([text](/{orgName}/path)). IntroducedbuildAlert()helper to ensure enrichment runs on all error return pathsCustomMarkdown): Internal links (paths starting with/) now use React Router'sLinkfor client-side navigation; external links continue to open in a new tabCan this PR break any existing features. If yes, please list possible items. If no, please explain why. (PS: Admins do not merge the PR without this section filled)
enrichMarkdownLinksis a no-op when messages don't contain markdown links.CustomMarkdown's external link behavior is unchanged — only links starting with/use the new React Router path.MissingEnvExceptionhad zero usages.Database Migrations
Env Config
Relevant Docs
Related Issues or PRs
Dependencies Versions
Notes on Testing
Screenshots
Checklist
I have read and understood the Contribution Guidelines.