[Devportal] Fix several consistency issues in devportal REST APIs#2660
Conversation
|
Warning Review limit reached
Next review available in: 27 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughThe PR standardizes Developer Portal REST contracts around ChangesDeveloper Portal contract alignment
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
portals/developer-portal/src/services/devportalService.js (1)
78-86: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winDo not forward raw error messages to the client.
Passing
error.messagedirectly intoutil.sendErrorrisks exposing internal details (such as database queries, stack traces, or file system paths) to the client. As per coding guidelines, raw errors must not be forwarded to the client response.Please replace
error.messagewith a generic, safe user-facing message like'Not Found'or'An error occurred while fetching content'. Alternatively, if this is meant to be handled globally, pass the error tonext(error)(after addingnextto the function signature).🛡️ Proposed fix to send a safe generic error message
logger.error('Error while fetching organization content', { error: error.message, stack: error.stack, orgId: req.orgId, viewId: req.params.viewId }); - util.sendError(res, 404, error.message); + util.sendError(res, 404, 'Content not found or an error occurred'); }🤖 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/developer-portal/src/services/devportalService.js` around lines 78 - 86, Update the catch block in the organization content fetch method to stop passing error.message to util.sendError; return a safe generic user-facing message such as “Not Found” or the existing service error wording, while retaining detailed error.message and stack only in logger.error.Source: Coding guidelines
🤖 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 `@docs/rest-apis/devportal/api-keys.md`:
- Around line 140-141: Remove the duplicate |status|error| row from each
Enumerated Values table at docs/rest-apis/devportal/api-keys.md lines 140-141
and 658-659, and docs/rest-apis/devportal/mcp-server-keys.md lines 140-141 and
658-659, leaving one status/error entry per table.
In `@portals/developer-portal/src/services/apiMetadataService.js`:
- Around line 82-93: Guard req.body.metadata before JSON.parse in the create
metadata branch, and apply the same change in updateAPIMetadata. When metadata
is missing, route through the existing Sequelize.ValidationError 400-handling
path instead of allowing JSON.parse to throw a SyntaxError; preserve normal
parsing and handle assignment for supplied metadata.
In `@portals/developer-portal/src/services/keyManagerService.js`:
- Around line 169-171: Update both error-response branches in
keyManagerService.js at lines 169-171 and 212-214 to stop returning
error.message; use the same safe generic message, “Invalid payload format or
validation failed.”, in each util.sendError call while preserving the existing
400 status and response flow.
- Around line 172-173: Update createKeyManager, updateKeyManager,
getKeyManagers, getKeyManager, and deleteKeyManager in
portals/developer-portal/src/services/keyManagerService.js at lines 172-173,
215-216, 234, 254-255, and 274-275 to accept Express’s next callback; in each
unexpected-error catch block, remove the local logger and manual 500 response
and return next(error) so centralized middleware handles the error.
- Around line 166-168: Replace the create-flow UniqueConstraintError response in
keyManagerService with a generic duplicate key-manager message that does not
interpolate req.body.id, matching the existing message used by the update flow.
In `@portals/developer-portal/src/utils/util.js`:
- Around line 311-312: Update the limit and offset calculations in the request
pagination logic to clamp parsed values at zero with Math.max(), while
preserving the existing limit ceiling of 100 and defaults of 20 and 0. Ensure
negative query parameters cannot reach Array.prototype.slice() and trigger
negative-index behavior.
---
Outside diff comments:
In `@portals/developer-portal/src/services/devportalService.js`:
- Around line 78-86: Update the catch block in the organization content fetch
method to stop passing error.message to util.sendError; return a safe generic
user-facing message such as “Not Found” or the existing service error wording,
while retaining detailed error.message and stack only in logger.error.
🪄 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: 381863ba-e791-469a-bb2b-8b309987c359
📒 Files selected for processing (60)
docs/rest-apis/devportal/api-content.mddocs/rest-apis/devportal/api-keys.mddocs/rest-apis/devportal/api-workflows.mddocs/rest-apis/devportal/apis.mddocs/rest-apis/devportal/applications.mddocs/rest-apis/devportal/key-managers.mddocs/rest-apis/devportal/labels.mddocs/rest-apis/devportal/mcp-server-content.mddocs/rest-apis/devportal/mcp-server-keys.mddocs/rest-apis/devportal/mcp-servers.mddocs/rest-apis/devportal/organizations.mddocs/rest-apis/devportal/schemas.mddocs/rest-apis/devportal/subscription-plans.mddocs/rest-apis/devportal/subscriptions.mddocs/rest-apis/devportal/views.mddocs/rest-apis/devportal/webhook-events.mddocs/rest-apis/devportal/webhook-subscribers.mdportals/developer-portal/docs/devportal-openapi-spec-v0.9.yamlportals/developer-portal/docs/publish-apis/api-content-and-docs.mdportals/developer-portal/it/rest-api/apis/artifact-zip-apis.spec.jsportals/developer-portal/it/rest-api/apis/graphql-apis.spec.jsportals/developer-portal/it/rest-api/apis/rest-apis.spec.jsportals/developer-portal/it/rest-api/apis/soap-apis.spec.jsportals/developer-portal/it/rest-api/apis/websocket-apis.spec.jsportals/developer-portal/it/rest-api/apis/websub-apis.spec.jsportals/developer-portal/it/rest-api/mcp-servers/mcp-servers.spec.jsportals/developer-portal/it/rest-api/subscriptions/subscriptions.spec.jsportals/developer-portal/it/rest-api/support/fixtures.jsportals/developer-portal/samples/mcps/travel-assistant-mcp-v1/definition.yamlportals/developer-portal/scripts/drift_check.jsportals/developer-portal/src/controllers/apiContentController.jsportals/developer-portal/src/controllers/apiKeyController.jsportals/developer-portal/src/controllers/apiWorkflowsController.jsportals/developer-portal/src/controllers/devportalController.jsportals/developer-portal/src/controllers/viewConfigureController.jsportals/developer-portal/src/controllers/webhookAdminController.jsportals/developer-portal/src/defaultContent/pages/api-keys/page.hbsportals/developer-portal/src/defaultContent/pages/api-landing/page.hbsportals/developer-portal/src/defaultContent/pages/api-landing/partials/api-detail-banner.hbsportals/developer-portal/src/defaultContent/pages/docs/partials/api-doc.hbsportals/developer-portal/src/defaultContent/pages/mcp-landing/page.hbsportals/developer-portal/src/defaultContent/pages/mcp-landing/partials/mcp-detail-banner.hbsportals/developer-portal/src/defaultContent/pages/mcp/partials/mcp-listing.hbsportals/developer-portal/src/dto/apiDto.jsportals/developer-portal/src/dto/mcpServerDto.jsportals/developer-portal/src/pages/settings/page.hbsportals/developer-portal/src/services/adminService.jsportals/developer-portal/src/services/apiMetadataService.jsportals/developer-portal/src/services/apiWorkflowService.jsportals/developer-portal/src/services/devportalService.jsportals/developer-portal/src/services/keyManagerService.jsportals/developer-portal/src/services/mcpRegistryService.jsportals/developer-portal/src/services/mcpServerService.jsportals/developer-portal/src/services/sampleSeederService.jsportals/developer-portal/src/services/subscriptionService.jsportals/developer-portal/src/services/webhookSubscriberService.jsportals/developer-portal/src/utils/constants.jsportals/developer-portal/src/utils/sampleApiLoader.jsportals/developer-portal/src/utils/util.jstests/integration-e2e/steps_devportal_test.go
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/rest-apis/devportal/subscription-plans.md (1)
61-66: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winAlign the paginated response examples with their metadata. Each example reports one returned item but 42 total records on an unfiltered first page with a limit of 20.
docs/rest-apis/devportal/subscription-plans.md#L61-L66: settotalto1, show thenamefilter, or provide a representative page matching the metadata.docs/rest-apis/devportal/key-managers.md#L186-L191: makecount,list, andpaginationdescribe the same page.docs/rest-apis/devportal/views.md#L166-L171: makecount,list, andpaginationdescribe the same page.🤖 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 `@docs/rest-apis/devportal/subscription-plans.md` around lines 61 - 66, Align all paginated response examples so count, list contents, and pagination metadata describe the same page: update docs/rest-apis/devportal/subscription-plans.md lines 61-66 by setting total to 1, adding the name filter, or supplying a representative page; update docs/rest-apis/devportal/key-managers.md lines 186-191 and docs/rest-apis/devportal/views.md lines 166-171 so their count, list, and pagination values are consistent.
🤖 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 `@docs/rest-apis/devportal/organization-content.md`:
- Line 74: Change the “Enumerated Values” heading from level four to level three
so it matches the preceding h3 section and satisfies the document’s heading
hierarchy.
In `@portals/developer-portal/src/services/devportalService.js`:
- Line 85: Update the catch-block util.sendError call in the relevant devportal
service function to return HTTP 500 with an “Internal Server Error” message for
unhandled exceptions, preserving the existing JSON error shape. Add an explicit
return statement to match the function’s other error branches.
---
Outside diff comments:
In `@docs/rest-apis/devportal/subscription-plans.md`:
- Around line 61-66: Align all paginated response examples so count, list
contents, and pagination metadata describe the same page: update
docs/rest-apis/devportal/subscription-plans.md lines 61-66 by setting total to
1, adding the name filter, or supplying a representative page; update
docs/rest-apis/devportal/key-managers.md lines 186-191 and
docs/rest-apis/devportal/views.md lines 166-171 so their count, list, and
pagination values are consistent.
🪄 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: 5d1fe2f7-81ec-494e-866f-f87e7cd4e511
📒 Files selected for processing (23)
docs/rest-apis/devportal/api-content.mddocs/rest-apis/devportal/api-keys.mddocs/rest-apis/devportal/apis.mddocs/rest-apis/devportal/application-keys.mddocs/rest-apis/devportal/applications.mddocs/rest-apis/devportal/key-managers.mddocs/rest-apis/devportal/labels.mddocs/rest-apis/devportal/mcp-server-content.mddocs/rest-apis/devportal/mcp-server-keys.mddocs/rest-apis/devportal/mcp-servers.mddocs/rest-apis/devportal/organization-content.mddocs/rest-apis/devportal/organizations.mddocs/rest-apis/devportal/schemas.mddocs/rest-apis/devportal/subscription-plans.mddocs/rest-apis/devportal/subscriptions.mddocs/rest-apis/devportal/views.mddocs/rest-apis/devportal/webhook-subscribers.mdportals/developer-portal/docs/devportal-openapi-spec-v0.9.yamlportals/developer-portal/src/services/apiMetadataService.jsportals/developer-portal/src/services/devportalService.jsportals/developer-portal/src/services/keyManagerService.jsportals/developer-portal/src/utils/util.jstests/integration-e2e/steps_devportal_test.go
💤 Files with no reviewable changes (1)
- docs/rest-apis/devportal/schemas.md
🚧 Files skipped from review as they are similar to previous changes (8)
- tests/integration-e2e/steps_devportal_test.go
- portals/developer-portal/src/utils/util.js
- portals/developer-portal/src/services/keyManagerService.js
- docs/rest-apis/devportal/mcp-server-keys.md
- docs/rest-apis/devportal/subscriptions.md
- docs/rest-apis/devportal/api-keys.md
- portals/developer-portal/src/services/apiMetadataService.js
- portals/developer-portal/docs/devportal-openapi-spec-v0.9.yaml
Purpose
Fix several consistency issues in devportal REST APIs
Approach
api*format with a generalized attributes.apiTitlein api/mcp creation ->title, so it will make sense for both apis and mcpsapiIdinPOST /subscriptionrequestBody toartifactIdsince the endpoint can be used with both apis and mcpsSamples
N/A