feat: add try-it-out execution - #74
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds Swagger endpoint server URL extraction, a validated ChangesTry It Out Flow
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant SwaggerViewer
participant TryItOutRoute
participant DNSResolver
participant NodeHttpClient
User->>SwaggerViewer: Enter parameters and execute endpoint
SwaggerViewer->>TryItOutRoute: POST composed request
TryItOutRoute->>DNSResolver: Validate destination address
DNSResolver-->>TryItOutRoute: Return resolved public address
TryItOutRoute->>NodeHttpClient: Proxy validated request
NodeHttpClient-->>TryItOutRoute: Return response
TryItOutRoute-->>SwaggerViewer: Return response envelope
SwaggerViewer-->>User: Display status, headers, duration, and body
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (3)
src/components/swagger-viewer/swagger-viewer.tsx (1)
102-166: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider extracting Try-It-Out logic into its own module.
swagger-viewer.tsxis accumulating several independent concerns in one file: schema-list rendering,EndpointPanel, path/URL-building helpers (applyPathParameters,buildTryItOutRequest,joinUrl,getParameterKey), and the entireTryItOutPanel/TryItOutResponseDetailsimplementation. Splitting the try-it-out pieces into a dedicated file (e.g.try-it-out-panel.tsx) would keep this file cohesive and easier to navigate as it continues to grow across this stacked PR series.As per path instructions,
src/**/*: "Flag code smells: God-objects, duplicate code chunks, unused files or exports."Also applies to: 201-260, 295-301, 389-548
🤖 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 `@src/components/swagger-viewer/swagger-viewer.tsx` around lines 102 - 166, Extract the Try-It-Out functionality from swagger-viewer.tsx into a dedicated module, including applyPathParameters, buildTryItOutRequest, joinUrl, getParameterKey, TryItOutPanel, and TryItOutResponseDetails. Update EndpointPanel and any other callers to import the moved symbols, preserving existing behavior and exports while leaving schema-list rendering and unrelated Swagger viewer logic in swagger-viewer.tsx.Source: Path instructions
src/components/swagger-viewer/swagger-viewer.test.tsx (1)
126-222: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMissing test for remote 4xx/5xx displayed in the response panel.
This new test only covers a successful (200) proxied response. Given the explicit product requirement that remote 4xx/5xx must render inside the response section rather than as an error toast, it'd be worth adding a case where the mocked
/api/try-it-outresponse contains e.g.status: 404and asserting it renders viaTryItOutResponseDetails(red chip) without triggeringshowToast.As per path instructions: "HTTP 4xx and 5xx status codes returned from user-executed API requests (Try-It-Out feature) must be displayed within the response section of the UI, not surfaced as application-level error toasts."
🤖 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 `@src/components/swagger-viewer/swagger-viewer.test.tsx` around lines 126 - 222, The SwaggerViewer tests lack coverage for proxied remote 4xx/5xx responses. Add a test alongside the existing endpoint execution test that mocks /api/try-it-out with a non-OK response such as status 404, executes the endpoint, and asserts TryItOutResponseDetails renders the status and response body in the response section with the red status treatment; also spy on showToast and verify it is not called.Source: Path instructions
src/utils/swagger-editor/get-swagger-endpoints.ts (1)
133-156: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
getDefaultServerUrldoesn't validate absolute URLs or respect path/operation-level overrides.Two spec gaps here:
- OpenAPI 3
servers[].urlmay be a relative reference (resolved against the document's hosting location), which is valid per spec. This function returns it as-is; downstreambuildTryItOutRequest/TryItOutPaneldoesnew URL(joinUrl(serverUrl, path)), which throws for a relative string like/v2, causing a generic "invalid URL" error for specs that legitimately use relative servers.- Per the OpenAPI spec,
serverscan also be declared at the Path Item and Operation level, and those levels override the document-level servers. This implementation only readsschema.servers(document root), so any endpoint with a path/operation-level server override will incorrectly show the document-level (or wrong) URL.Both are edge cases with a safe fallback (the server URL field is user-editable), so functionality isn't fully broken, but this can silently mislead users composing requests.
♻️ Suggested guard for relative URLs (path/operation overrides need broader changes to plumb per-operation server data through)
if (Array.isArray(servers)) { const firstServer = servers.find((server) => isRecord(server) && getString(server.url)); if (isRecord(firstServer)) { - return getString(firstServer.url); + const url = getString(firstServer.url); + + try { + return new URL(url).toString(); + } catch { + return ''; // relative server URL - fall back to manual entry + } } }🤖 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 `@src/utils/swagger-editor/get-swagger-endpoints.ts` around lines 133 - 156, Update getDefaultServerUrl to handle relative OpenAPI 3 server URLs safely: validate or resolve servers[].url against an available document base URL before returning it, with a safe fallback for unresolved relative references so downstream buildTryItOutRequest and TryItOutPanel do not pass invalid input to new URL. Also trace endpoint construction to propagate Path Item and Operation-level servers, ensuring those overrides take precedence over document-level servers.
🤖 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 `@src/app/api/try-it-out/route.ts`:
- Around line 45-51: Replace the hardcoded English error messages in the
try-it-out route’s error responses with stable error codes, including the
invalid payload, missing URL/method, and execution failure cases. Update the
client error handling in swagger-viewer.tsx to map those codes through the
appropriate i18n keys, while retaining a translated fallback for unknown errors.
- Around line 36-43: Add an AbortController-based timeout around the fetch in
the route handler, passing its signal to fetch and clearing the timeout after
completion; handle timeout failures consistently with other request errors. Also
tighten the existing URL validation to reject arbitrary http(s) destinations and
permit only the intended safe targets.
- Around line 22-54: Update POST and its URL/request helpers, including getUrl,
to prevent SSRF: resolve the initial hostname and reject loopback, private,
link-local, and other non-public IP ranges, use fetch with redirect set to
manual, and validate every Location redirect target before following it.
Preserve existing validation and return a suitable client error for blocked
destinations.
In `@src/components/swagger-viewer/swagger-viewer.tsx`:
- Around line 429-434: Replace the raw string handling in the swagger viewer’s
request failure logic with a coordinated error-code flow: update the route
handler to return a stable error code instead of localized text, then map that
code through the client’s translation function before calling setError in the
!result.ok branch. Preserve a translated fallback such as t('tryItOutFailed')
for unknown or malformed errors, and update related error handling consistently
on both sides.
In `@src/i18n/messages/ru.json`:
- Line 50: Translate the tryItOutTitle value in the Russian messages resource
instead of leaving the English text “Try it out”; use a natural Russian
equivalent consistent with the surrounding translations.
---
Nitpick comments:
In `@src/components/swagger-viewer/swagger-viewer.test.tsx`:
- Around line 126-222: The SwaggerViewer tests lack coverage for proxied remote
4xx/5xx responses. Add a test alongside the existing endpoint execution test
that mocks /api/try-it-out with a non-OK response such as status 404, executes
the endpoint, and asserts TryItOutResponseDetails renders the status and
response body in the response section with the red status treatment; also spy on
showToast and verify it is not called.
In `@src/components/swagger-viewer/swagger-viewer.tsx`:
- Around line 102-166: Extract the Try-It-Out functionality from
swagger-viewer.tsx into a dedicated module, including applyPathParameters,
buildTryItOutRequest, joinUrl, getParameterKey, TryItOutPanel, and
TryItOutResponseDetails. Update EndpointPanel and any other callers to import
the moved symbols, preserving existing behavior and exports while leaving
schema-list rendering and unrelated Swagger viewer logic in swagger-viewer.tsx.
In `@src/utils/swagger-editor/get-swagger-endpoints.ts`:
- Around line 133-156: Update getDefaultServerUrl to handle relative OpenAPI 3
server URLs safely: validate or resolve servers[].url against an available
document base URL before returning it, with a safe fallback for unresolved
relative references so downstream buildTryItOutRequest and TryItOutPanel do not
pass invalid input to new URL. Also trace endpoint construction to propagate
Path Item and Operation-level servers, ensuring those overrides take precedence
over document-level servers.
🪄 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 Plus
Run ID: 4b40644e-ab2b-418a-847b-7684d09fcb76
📒 Files selected for processing (8)
src/app/api/try-it-out/route.test.tssrc/app/api/try-it-out/route.tssrc/components/swagger-viewer/swagger-viewer.test.tsxsrc/components/swagger-viewer/swagger-viewer.tsxsrc/i18n/messages/en.jsonsrc/i18n/messages/ru.jsonsrc/utils/swagger-editor/get-swagger-endpoints.test.tssrc/utils/swagger-editor/get-swagger-endpoints.ts
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/app/api/try-it-out/route.test.ts (1)
102-121: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winGood coverage for blocking private destinations and blocked redirect targets. Consider adding an IPv6 case (e.g. a non-
fe80::-literal link-local address) once the classification gap inisPublicIpis fixed, to lock in the corrected behavior.Also applies to: 169-191
🤖 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 `@src/app/api/try-it-out/route.test.ts` around lines 102 - 121, Add an IPv6 link-local destination test alongside the existing private-destination and blocked-redirect tests in the try-it-out route test suite. Use a non-fe80:: IPv6 literal that should be classified as private, assert POST returns status 400 with errorCode "blockedUrl", and verify fetch is not called; include the corresponding redirect-target coverage if applicable.
🤖 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 `@src/app/api/try-it-out/route.ts`:
- Around line 220-236: Expand the IPv6 link-local validation in the ipVersion
=== 6 branch to cover the entire fe80::/10 range, not only addresses starting
with “fe80:”. Parse the first IPv6 hextet or use an equivalent range check and
reject values from 0xfe80 through 0xfebf, while preserving the existing handling
in the address normalization and isPublicIp logic.
- Around line 74-108: Replace the separate DNS preflight in assertPublicUrl and
unpinned fetchWithTimeout usage in fetchValidatedUrl with an outbound
connector/dispatcher that resolves the hostname once, validates every resolved
address with isPublicIp, and pins that validated address for the actual
connection; preserve protocol and redirect checks and apply the same protection
on each redirect.
---
Nitpick comments:
In `@src/app/api/try-it-out/route.test.ts`:
- Around line 102-121: Add an IPv6 link-local destination test alongside the
existing private-destination and blocked-redirect tests in the try-it-out route
test suite. Use a non-fe80:: IPv6 literal that should be classified as private,
assert POST returns status 400 with errorCode "blockedUrl", and verify fetch is
not called; include the corresponding redirect-target coverage if applicable.
🪄 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 Plus
Run ID: d26b721e-cba7-416b-b3d8-7b70065af58d
📒 Files selected for processing (6)
src/app/api/try-it-out/route.test.tssrc/app/api/try-it-out/route.tssrc/components/swagger-viewer/swagger-viewer.test.tsxsrc/components/swagger-viewer/swagger-viewer.tsxsrc/i18n/messages/en.jsonsrc/i18n/messages/ru.json
✅ Files skipped from review due to trivial changes (2)
- src/i18n/messages/ru.json
- src/i18n/messages/en.json
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/swagger-viewer/swagger-viewer.tsx
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/app/api/try-it-out/route.ts (2)
91-104: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winRedirects re-send
Authorization/Cookieto the redirect target, including cross-host hops.
init(with the user's original headers) is reused unchanged on every redirect hop. If the initial origin responds with a302to a different host, the sameAuthorization/Cookieheaders are replayed to that host. Each hop is IP-validated, but a redirect controlled by the target can still exfiltrate credentials the user entered for their own API. Standard clients strip credential headers on cross-origin redirects.Consider dropping sensitive headers when
new URL(location, currentUrl).host !== currentUrl.host.🤖 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 `@src/app/api/try-it-out/route.ts` around lines 91 - 104, fetchValidatedUrl must not forward credentials across origins. When resolving each redirect in fetchValidatedUrl, compare the destination URL’s host with currentUrl.host; if they differ, create redirect request options with Authorization and Cookie headers removed before calling requestWithPinnedIp, while preserving other headers and same-host credentials.
265-277: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winResponse body is buffered without a size limit.
Buffer.concat(chunks)accumulates the entire upstream body in memory. A large or malicious origin can grow this unbounded within the 15s window and exhaust process memory on this shared route. Consider tracking cumulative length and destroying the request once a cap is exceeded.🤖 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 `@src/app/api/try-it-out/route.ts` around lines 265 - 277, Limit response buffering in the response handling around the `response.on('data')` and `response.on('end')` callbacks. Track cumulative chunk size, enforce a reasonable maximum body size, and destroy or abort the request and reject the promise when the cap is exceeded; only call `Buffer.concat(chunks)` after confirming the limit was not exceeded.
🤖 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 `@src/app/api/try-it-out/route.ts`:
- Around line 263-278: Add an error listener to the response stream in the
Promise within the try-it-out request handler, using response.on('error',
reject) alongside the existing data and end handlers. Keep
requestMessage.on('error') for client-side failures so both request and response
stream errors settle the Promise.
---
Nitpick comments:
In `@src/app/api/try-it-out/route.ts`:
- Around line 91-104: fetchValidatedUrl must not forward credentials across
origins. When resolving each redirect in fetchValidatedUrl, compare the
destination URL’s host with currentUrl.host; if they differ, create redirect
request options with Authorization and Cookie headers removed before calling
requestWithPinnedIp, while preserving other headers and same-host credentials.
- Around line 265-277: Limit response buffering in the response handling around
the `response.on('data')` and `response.on('end')` callbacks. Track cumulative
chunk size, enforce a reasonable maximum body size, and destroy or abort the
request and reject the promise when the cap is exceeded; only call
`Buffer.concat(chunks)` after confirming the limit was not exceeded.
🪄 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 Plus
Run ID: d6998ebf-0e31-4fa6-ae77-0809c43a57d4
📒 Files selected for processing (2)
src/app/api/try-it-out/route.test.tssrc/app/api/try-it-out/route.ts
Summary
Closes #66
Adds Try-it-out request execution to the Swagger Viewer on top of the endpoint details work from #72.
What Changed
Added server-side request execution route:
Extended endpoint normalization with server URL extraction:
servers[0].urlschemes + host + basePathAdded Try-it-out UI in Swagger Viewer:
Added EN/RU translations for Try-it-out labels.
Added tests for:
Scope
This PR only implements Try-it-out request execution.
Not included in this PR:
These belong to follow-up tasks.
Verification
story/4-endpoint-detailsstory/4-endpoint-listdevelopPR Dependency
This is a stacked PR and should be reviewed after #72.
Base branch:
story/4-endpoint-detailsHead branch:
story/4-try-it-outNotes
The implementation preserves the endpoint list/details changes from #69 and #72 and does not modify unrelated Editor/Auth/History behavior.
Summary by CodeRabbit
New Features
Bug Fixes
Tests
serverUrl.