Show specific backend error description on AI provider save (#5028)#1347
Show specific backend error description on AI provider save (#5028)#1347Tharsanan1 wants to merge 5 commits into
Conversation
…028) Extract response.body.description from the backend error so the admin UI displays the precise validation message (e.g. invalid JSONPath expression) instead of a generic catch-all error text. Fixes wso2/api-manager#5028
Tharsanan1
requested review from
Krishanx92,
lasanthaS,
piyumaldk and
tharikaGitHub
as code owners
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe Add/Edit AI Service Provider form's save error handler now prefers ChangesAI Service Provider Error Messaging
sequenceDiagram
participant AddEditAiServiceProvider
participant Backend
participant Alert
AddEditAiServiceProvider->>Backend: save request
Backend-->>AddEditAiServiceProvider: error (response.body.description / message)
AddEditAiServiceProvider->>Alert: display response.body.description or message or localized generic error
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add 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 |
|
@coderabbitai please review |
|
✅ Actions performedReview triggered.
|
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@portals/admin/src/main/webapp/source/src/app/components/AiServiceProviders/AddEditAiServiceProvider.jsx`:
- Around line 432-435: In the AddEditAiServiceProvider component, the error
branch uses Alert.error(response.body.description) even when response.body
exists but has no description; update the condition in the error handling so it
checks for a real description (e.g., response && response.body &&
response.body.description) or simply use optional chaining/fallback like
Alert.error(response?.body?.description || message) so the generic message
fallback is used when description is absent; ensure Alert.error is never called
with undefined.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1b5523b0-1f38-47f9-9f4d-a3ee410f479c
📒 Files selected for processing (1)
portals/admin/src/main/webapp/source/src/app/components/AiServiceProviders/AddEditAiServiceProvider.jsx
…s absent Use optional chaining so the generic message fallback is reached when response.body exists but description is missing, preventing Alert.error(undefined). Addresses CodeRabbit comment: https://github.com/wso2/apim-apps/pull/1347/changes#r3217291794 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Tharsanan1
requested review from
HeshanSudarshana,
PasanT9,
RakhithaRR and
hisanhunais
as code owners
|
@coderabbitai please review |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Fetch the PR diff
gh pr view 1347 --repo wso2/apim-apps --json files,additions,deletions,title,body
# Get the diff
gh pr diff 1347 --repo wso2/apim-appsLength of output: 2843 🏁 Script executed: #!/bin/bash
# Check how other catch blocks in the AiServiceProviders component handle error responses
cat "portals/admin/src/main/webapp/source/src/app/components/AiServiceProviders/AddEditAiServiceProvider.jsx"
# Check patterns across other components for response.body usage in catch blocks
rg -n "response\.body\.description\|response\.body\.message\|e\.response\.body" \
portals/admin/src/main/webapp/source/src/app/components/ \
--type jsx -A2 -B2 2>/dev/null || \
rg -n "response\.body\.description\|response\.body\.message\|e\.response\.body" \
portals/admin/src/main/webapp/source/src/app/components/ \
-A2 -B2
# Look for similar error handling patterns in other admin portal components
rg -n "Alert\.error.*description\|response\?\.body\|\.body\.description" \
portals/admin/src/main/webapp/source/src/app/components/ \
-A2 -B2 | head -80Length of output: 61720
Review of
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
portals/admin/src/main/webapp/source/src/app/components/AiServiceProviders/AddEditAiServiceProvider.jsx (1)
441-441: 💤 Low valueConsider removing duplicate
setSaving(false)(pre-existing).Line 438 already calls
setSaving(false)in the catch block, and line 429 redirects viahistory.pushon success, making this line effectively unreachable or redundant. While this issue predates the current PR, it could be cleaned up for better code clarity.♻️ Proposed cleanup
setSaving(false); } - - setSaving(false); return true;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@portals/admin/src/main/webapp/source/src/app/components/AiServiceProviders/AddEditAiServiceProvider.jsx` at line 441, In AddEditAiServiceProvider.jsx remove the redundant setSaving(false) call that follows the success path (after history.push) because setSaving(false) is already called in the catch block and the success branch redirects via history.push; keep the setSaving(false) inside the catch block and any pre-existing error handling, and delete the duplicate call that is unreachable after history.push to clean up the component's save flow.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@portals/admin/src/main/webapp/source/src/app/components/AiServiceProviders/AddEditAiServiceProvider.jsx`:
- Around line 435-437: The current error handling in
AddEditAiServiceProvider.jsx checks backendDescription and message but leaves a
silent fail when both are falsy; update the conditional that currently has "else
if (message) { Alert.error(message); }" to add a final else branch that calls
Alert.error with a generic fallback (e.g., "An unexpected error occurred" or
"Unable to save provider, please try again") so users always receive feedback
when backendDescription and message are unavailable.
---
Nitpick comments:
In
`@portals/admin/src/main/webapp/source/src/app/components/AiServiceProviders/AddEditAiServiceProvider.jsx`:
- Line 441: In AddEditAiServiceProvider.jsx remove the redundant
setSaving(false) call that follows the success path (after history.push) because
setSaving(false) is already called in the catch block and the success branch
redirects via history.push; keep the setSaving(false) inside the catch block and
any pre-existing error handling, and delete the duplicate call that is
unreachable after history.push to clean up the component's save flow.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 126cdc53-6cc1-45ea-88ec-19f0519cc211
📒 Files selected for processing (1)
portals/admin/src/main/webapp/source/src/app/components/AiServiceProviders/AddEditAiServiceProvider.jsx
Tharsanan1
force-pushed
the
fix/issue-5028
branch
from
27b6952 to
382e6cf
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@portals/admin/src/main/webapp/source/src/app/components/AiServiceProviders/AddEditAiServiceProvider.jsx`:
- Around line 437-438: Replace the hardcoded Alert.error string in the
AddEditAiServiceProvider component with a localized message via
intl.formatMessage; locate the Alert.error call and pass the result of
intl.formatMessage(...) (using an appropriate message id like a new key for the
generic save error) so the fallback error message is retrieved from the
translations instead of being hardcoded.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ba00466f-4b2f-495e-a712-35b8f04fd7d9
📒 Files selected for processing (1)
portals/admin/src/main/webapp/source/src/app/components/AiServiceProviders/AddEditAiServiceProvider.jsx
…error message are unavailable Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Tharsanan1
force-pushed
the
fix/issue-5028
branch
from
382e6cf to
56fd7e8
Compare
|
@CodeRabbit please review and approve |
|
✅ Actions performedComments resolved and changes approved. |
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@portals/admin/src/main/webapp/site/public/locales/fr.json`:
- Line 297: The JSON key
"AiServiceProviders.AddEditAiServiceProvider.save.error.generic" is in English;
update its value to a proper French translation (for example: "Une erreur est
survenue lors de l'enregistrement du fournisseur de service d'IA.") so the
French locale file uses French text for this error message.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 10603f88-39c7-4f56-8bd4-035c08a5115f
📒 Files selected for processing (3)
portals/admin/src/main/webapp/site/public/locales/en.jsonportals/admin/src/main/webapp/site/public/locales/fr.jsonportals/admin/src/main/webapp/source/src/app/components/AiServiceProviders/AddEditAiServiceProvider.jsx
✅ Files skipped from review due to trivial changes (1)
- portals/admin/src/main/webapp/site/public/locales/en.json
🚧 Files skipped from review as they are similar to previous changes (1)
- portals/admin/src/main/webapp/source/src/app/components/AiServiceProviders/AddEditAiServiceProvider.jsx
2 participants
Issue
Fixes wso2/api-manager#5028
When saving an AI service provider with an invalid metadata identifier, the admin portal displayed a generic error message instead of the specific validation error returned by the backend (e.g., "Invalid JSONPath expression '[0-9]+' for attribute 'promptTokenCount'").
Root Cause
The catch block in
AddEditAiServiceProvider.jsxusede.message(the JavaScript Error object's message) instead of extracting thedescriptionfield from the API error response body. With the backend now returning a structured{ code, message, description }error, the description contains the actionable detail.What Was Changed
AddEditAiServiceProvider.jsx: Updated the catch block to extractresponse.body.description(falling back to the existing generic message if not present) so the UI shows the precise validation error returned by the backend.Verification
descriptionfield from the backend 400 response body.Related backend fix: wso2/carbon-apimgt#13833