[Docs] fix broken requests on API playground#1125
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughAdds richer error metadata and inference to API responses, mandates JSON bodies and Content-Type for POST/PUT/PATCH (including empty bodies), updates curl/JS/Python code-example generation to always include JSON bodies, and enhances the error UI with type badges and request duration. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 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 |
Greptile OverviewGreptile SummaryFixed broken API requests in the documentation playground by ensuring POST/PUT/PATCH requests always include a body when the OpenAPI specification defines a Additionally improved error handling by categorizing network errors (network/cors/timeout) and providing clearer, more actionable error messages to users. The UI now displays error type badges and duration information for failed requests. Key changes:
Confidence Score: 5/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant Playground as API Playground
participant API as API Server
User->>Playground: Click "Try it out"
alt Operation has requestBody
Playground->>Playground: Check operation.requestBody
Playground->>Playground: Build bodyData from bodyFields<br/>(filter empty values)
Playground->>Playground: Always set body = JSON.stringify(bodyData)
Playground->>Playground: Add Content-Type: application/json
end
Playground->>API: fetch(url, { method, headers, body })
alt Success Response
API-->>Playground: { status: 200, data: {...} }
Playground->>User: Display success response
else HTTP Error
API-->>Playground: { status: 4xx/5xx, error: {...} }
Playground->>User: Display error with status code
else Network Error
API--xPlayground: Network failure
Playground->>Playground: Categorize error type<br/>(network/cors/timeout)
Playground->>User: Display helpful error message<br/>with error type badge
end
|
![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}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/src/components/api/enhanced-api-page.tsx (1)
209-209: Useperformance.now()instead ofDate.now()for measuring elapsed time.As per coding guidelines,
performance.now()should be used for measuring elapsed time as it provides higher precision and is not affected by system clock changes.Proposed fix
- const startTime = Date.now(); + const startTime = performance.now();And update line 286:
- const endTime = Date.now(); + const endTime = performance.now();And update line 335:
- duration: Date.now() - startTime, + duration: performance.now() - startTime,Note: The duration values will now be in milliseconds with sub-millisecond precision (e.g.,
123.456instead of123). You may want to round them for display:Math.round(performance.now() - startTime).
🧹 Nitpick comments (4)
docs/src/components/api/enhanced-api-page.tsx (4)
30-34: Theurlfield inerrorDetailsis never populated.The
errorDetails.urlproperty is defined but never assigned a value when setting error state (see lines 330-333). Either populate it or remove it from the type definition.Proposed fix
If you want to keep the URL, populate it in the error handler:
errorDetails: { + url: url, method: method.toUpperCase(), type: errorType, },Or remove it from the type if not needed:
errorDetails?: { - url?: string, method?: string, type?: 'network' | 'cors' | 'timeout' | 'unknown', },
387-396: Avoid.catch(console.error)- userunAsynchronouslyorrunAsynchronouslyWithAlert.Per coding guidelines, promise errors should be handled using the project's established error handling utilities rather than
.catch(error => console.error(...)).Proposed fix
onExecute={() => { - // eslint-disable-next-line no-restricted-syntax - executeRequest(operation, path, method) - .catch(error => console.error('Failed to execute request:', error)); + runAsynchronouslyWithAlert(executeRequest(operation, path, method)); }} onCopy={(text: string) => { - // eslint-disable-next-line no-restricted-syntax - copyToClipboard(text) - .catch(error => console.error('Failed to copy to clipboard:', error)); + runAsynchronously(copyToClipboard(text)); }}You'll need to import
runAsynchronouslyand/orrunAsynchronouslyWithAlertfrom the appropriate module.
741-747: Same issue: Avoid.catch(console.error).This follows the same problematic pattern flagged earlier. Use
runAsynchronouslyfor consistency.Proposed fix
onClick={() => { - // eslint-disable-next-line no-restricted-syntax - handleCopy(getCodeExample()) - .catch(error => { - console.error('Failed to copy code example', error); - }); + runAsynchronously(handleCopy(getCodeExample())); }}
200-206: Consider using the project's error handling utility.While try-catch is acceptable here for handling clipboard API failures gracefully, the
console.errorcould be replaced with the project's standard error handling approach for consistency.
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
| errorDetails: { | ||
| method: method.toUpperCase(), | ||
| type: errorType, | ||
| }, |
There was a problem hiding this comment.
The errorDetails type defines a url field (line 31), and the PR description states that error responses include "URL, HTTP method, and error type classification," but the implementation never populates the URL field.
View Details
📝 Patch Details
diff --git a/docs/src/components/api/enhanced-api-page.tsx b/docs/src/components/api/enhanced-api-page.tsx
index 90a6fd68..33a7fb6a 100644
--- a/docs/src/components/api/enhanced-api-page.tsx
+++ b/docs/src/components/api/enhanced-api-page.tsx
@@ -328,6 +328,7 @@ export function EnhancedAPIPage({ document, operations, description }: EnhancedA
loading: false,
error: errorMessage,
errorDetails: {
+ url: url,
method: method.toUpperCase(),
type: errorType,
},
Analysis
Missing URL field in error details metadata for API playground
What fails: EnhancedAPIPage in docs/src/components/api/enhanced-api-page.tsx defines an errorDetails type with an optional url field (line 30), but the catch block handler (lines 330-333) only populates method and type fields, leaving url undefined despite it being available in scope.
How to reproduce:
- In the API playground component, trigger a network error (e.g., fetch fails due to network issue)
- The error response object's
errorDetails.urlproperty will be undefined - Expected:
errorDetails.urlshould contain the constructed request URL with all path parameters and query parameters resolved
Current behavior: errorDetails object contains only method and type:
errorDetails: {
method: method.toUpperCase(),
type: errorType,
}Expected behavior: errorDetails object should include the URL:
errorDetails: {
url: url,
method: method.toUpperCase(),
type: errorType,
}Root cause: The url variable is constructed and available in the executeRequest function (defined at line 236 in the try block), with path parameters and query parameters resolved before the fetch call. However, when error handling occurs in the catch block, the URL field was not included in the errorDetails metadata object despite being defined in the type schema, suggesting incomplete implementation of the feature for capturing comprehensive error context.
Impact: While the URL field is not currently rendered in the UI, including it in the error metadata would enable future debugging capabilities and align with the complete error context design indicated by the type definition.
2 participants
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.