fix: issues in api request viewer#1257
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughThis PR adds CURL code generation support to both standard and streaming API request viewers, and refactors the streaming viewer to propagate request metadata (method and query parameters) while removing responseField extraction in favor of printing full response objects per language. ChangesCURL Support and Streaming Refactor
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 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 |
|
There was a problem hiding this comment.
Pull request overview
This PR addresses the reported issues in ExecuteApiRequestViewer / ExecuteApiRequestStreamingViewer by adding a curl tab and making streaming snippet generation honor the provided HTTP method, query params, and optional body while simplifying streamed output logging.
Changes:
- Add
SupportedLanguage.CURLsupport for both non-streaming and streaming API executor viewers. - Update streaming snippet generation to respect
methodandqueryParams, and omit body-related code when no body is provided. - Simplify streaming output logging across SDK snippets to print the streamed item rather than a hardcoded field/type.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
| let code = `curl -X ${method} '${url}' \\\n`; | ||
| code += ` -H 'Content-Type: application/json' \\\n`; | ||
| code += ` -H 'Authorization: Bearer $FGA_BEARER_TOKEN'`; |
| code += `await foreach (var item in executor.ExecuteStreamingAsync<object, Dictionary<string, object>>(\n`; | ||
| code += ` request, "${operationName}"))\n{\n`; | ||
| code += ` Console.WriteLine(item.${field.charAt(0).toUpperCase() + field.slice(1)});`; | ||
| code += ` Console.WriteLine(item);`; |
| case SupportedLanguage.CURL: { | ||
| let url = `$FGA_API_URL${path}`; | ||
| if (pathParams) { | ||
| for (const [k, v] of Object.entries(pathParams)) { | ||
| url = url.split(`{${k}}`).join(v); | ||
| } | ||
| } | ||
| if (queryParams && Object.keys(queryParams).length > 0) { | ||
| const qs = Object.entries(queryParams) | ||
| .map(([k, v]) => `${k}=${v}`) | ||
| .join('&'); | ||
| url += `?${qs}`; | ||
| } | ||
| let code = `curl -X ${method} '${url}' \\\n`; | ||
| code += ` -H 'Content-Type: application/json' \\\n`; | ||
| code += ` -H 'Authorization: Bearer $FGA_BEARER_TOKEN'`; | ||
| if (body) { | ||
| code += ` \\\n -d '${JSON.stringify(body, null, 2)}'`; | ||
| } | ||
| if (responseExample) { | ||
| code += `\n\n# Response: ${responseExample}`; | ||
| } | ||
| return code; | ||
| } |
| } | ||
| let code = `curl -X ${method} '${url}' \\\n`; | ||
| code += ` -H 'Content-Type: application/json' \\\n`; | ||
| code += ` -H 'Authorization: Bearer $FGA_BEARER_TOKEN'`; |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/components/Docs/SnippetViewer/ExecuteApiRequestViewer.tsx (1)
322-323: 💤 Low valuePrefer a type alias over an empty extending interface.
The
eslint-disableis only needed because the body is empty. Atypealias expresses the same intent without suppressing the rule, and stays correct if the parent type changes.♻️ Proposed refactor
-// eslint-disable-next-line `@typescript-eslint/no-empty-object-type` -interface ExecuteApiRequestStreamingViewerOpts extends ExecuteApiRequestViewerOpts {} +type ExecuteApiRequestStreamingViewerOpts = ExecuteApiRequestViewerOpts;Note:
responseExampleis inherited via this type but the streaming generator (line 326) neither destructures nor emits it. If that is intentional, considerOmit<ExecuteApiRequestViewerOpts, 'responseExample'>so callers aren't misled into passing it.🤖 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/Docs/SnippetViewer/ExecuteApiRequestViewer.tsx` around lines 322 - 323, Replace the empty extending interface ExecuteApiRequestStreamingViewerOpts with a type alias to avoid disabling the eslint rule: declare ExecuteApiRequestStreamingViewerOpts = ExecuteApiRequestViewerOpts; also consider whether the streaming viewer actually uses responseExample (the streaming generator around the streaming viewer code does not destructure or emit responseExample) and if not change the alias to Omit<ExecuteApiRequestViewerOpts, 'responseExample'> so callers aren't misled into passing that prop.
🤖 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/components/Docs/SnippetViewer/ExecuteApiRequestViewer.tsx`:
- Around line 271-294: The curl branch in ExecuteApiRequestViewer.tsx builds
URLs and bodies without encoding or escaping: update the path param substitution
(the url mutation that does url.split(`{${k}}`).join(v)) to insert
encodeURIComponent(v) for each path value; when building the query string (the
map producing `${k}=${v}`) use encodeURIComponent for both key and value; and
make the body safe for shell by JSON.stringify(body, null, 2) and then escaping
single quotes inside that JSON (replace each ' with the shell-safe sequence
'"'"') before wrapping it in single quotes in the -d argument so the generated
curl string (the variable code) never breaks on apostrophes.
- Around line 465-485: In the SupportedLanguage.CURL branch inside
ExecuteApiRequestViewer.tsx (the CURL case building the `code` string for
method/path/url/body/responseExample), add the streaming-specific fixes: insert
`-N` (or `--no-buffer`) immediately after `curl` when building the command for
streaming endpoints, ensure any JSON body embedded in single quotes is
shell-safe by escaping single quotes (use .replace(/'/g, "'\\''") on
JSON.stringify(body, null, 2)), and emit the same `# Response:` comment using
`responseExample` as the non-streaming case so the field is not ignored;
optionally factor the duplicated CURL construction into a helper like
`buildCurlCommand({method, url, body, responseExample, streaming})` and call it
from both the non-streaming and streaming branches to keep behavior consistent.
---
Nitpick comments:
In `@src/components/Docs/SnippetViewer/ExecuteApiRequestViewer.tsx`:
- Around line 322-323: Replace the empty extending interface
ExecuteApiRequestStreamingViewerOpts with a type alias to avoid disabling the
eslint rule: declare ExecuteApiRequestStreamingViewerOpts =
ExecuteApiRequestViewerOpts; also consider whether the streaming viewer actually
uses responseExample (the streaming generator around the streaming viewer code
does not destructure or emit responseExample) and if not change the alias to
Omit<ExecuteApiRequestViewerOpts, 'responseExample'> so callers aren't misled
into passing that prop.
🪄 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: 60f03d65-850f-4e6d-81f9-f3757ec098d9
📒 Files selected for processing (1)
src/components/Docs/SnippetViewer/ExecuteApiRequestViewer.tsx
| case SupportedLanguage.CURL: { | ||
| let url = `$FGA_API_URL${path}`; | ||
| if (pathParams) { | ||
| for (const [k, v] of Object.entries(pathParams)) { | ||
| url = url.split(`{${k}}`).join(v); | ||
| } | ||
| } | ||
| if (queryParams && Object.keys(queryParams).length > 0) { | ||
| const qs = Object.entries(queryParams) | ||
| .map(([k, v]) => `${k}=${v}`) | ||
| .join('&'); | ||
| url += `?${qs}`; | ||
| } | ||
| let code = `curl -X ${method} '${url}' \\\n`; | ||
| code += ` -H 'Content-Type: application/json' \\\n`; | ||
| code += ` -H 'Authorization: Bearer $FGA_BEARER_TOKEN'`; | ||
| if (body) { | ||
| code += ` \\\n -d '${JSON.stringify(body, null, 2)}'`; | ||
| } | ||
| if (responseExample) { | ||
| code += `\n\n# Response: ${responseExample}`; | ||
| } | ||
| return code; | ||
| } |
There was a problem hiding this comment.
Generated curl commands are malformed for common inputs (no URL-encoding, unsafe single-quote body wrapping).
Three concrete defects in the new CURL branch will cause users who copy-paste the snippet to hit broken requests:
- Path parameters are not URL-encoded.
url.split(\{${k}}`).join(v)substitutes raw values, so anyvcontaining/,:,?,#`, space, etc. produces a malformed URL. - Query parameters are not URL-encoded.
${k}=${v}will silently break for values containing&,=,#,+, or spaces. - Body wrapped in single quotes will break on apostrophes.
-d '${JSON.stringify(body, null, 2)}'— if any JSON string value contains a single quote (e.g. a user name likeO'Brien), the shell single-quote string is terminated early and the resulting command is invalid. This is realistic for example data shown in docs.
🛠️ Proposed fix
case SupportedLanguage.CURL: {
let url = `$FGA_API_URL${path}`;
if (pathParams) {
for (const [k, v] of Object.entries(pathParams)) {
- url = url.split(`{${k}}`).join(v);
+ url = url.split(`{${k}}`).join(encodeURIComponent(v));
}
}
if (queryParams && Object.keys(queryParams).length > 0) {
const qs = Object.entries(queryParams)
- .map(([k, v]) => `${k}=${v}`)
+ .map(([k, v]) => `${encodeURIComponent(k)}=${encodeURIComponent(v)}`)
.join('&');
url += `?${qs}`;
}
let code = `curl -X ${method} '${url}' \\\n`;
code += ` -H 'Content-Type: application/json' \\\n`;
code += ` -H 'Authorization: Bearer $FGA_BEARER_TOKEN'`;
if (body) {
- code += ` \\\n -d '${JSON.stringify(body, null, 2)}'`;
+ // Escape single quotes so the shell-quoted -d body survives apostrophes in example data.
+ const json = JSON.stringify(body, null, 2).replace(/'/g, `'\\''`);
+ code += ` \\\n -d '${json}'`;
}
if (responseExample) {
code += `\n\n# Response: ${responseExample}`;
}
return code;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| case SupportedLanguage.CURL: { | |
| let url = `$FGA_API_URL${path}`; | |
| if (pathParams) { | |
| for (const [k, v] of Object.entries(pathParams)) { | |
| url = url.split(`{${k}}`).join(v); | |
| } | |
| } | |
| if (queryParams && Object.keys(queryParams).length > 0) { | |
| const qs = Object.entries(queryParams) | |
| .map(([k, v]) => `${k}=${v}`) | |
| .join('&'); | |
| url += `?${qs}`; | |
| } | |
| let code = `curl -X ${method} '${url}' \\\n`; | |
| code += ` -H 'Content-Type: application/json' \\\n`; | |
| code += ` -H 'Authorization: Bearer $FGA_BEARER_TOKEN'`; | |
| if (body) { | |
| code += ` \\\n -d '${JSON.stringify(body, null, 2)}'`; | |
| } | |
| if (responseExample) { | |
| code += `\n\n# Response: ${responseExample}`; | |
| } | |
| return code; | |
| } | |
| case SupportedLanguage.CURL: { | |
| let url = `$FGA_API_URL${path}`; | |
| if (pathParams) { | |
| for (const [k, v] of Object.entries(pathParams)) { | |
| url = url.split(`{${k}}`).join(encodeURIComponent(v)); | |
| } | |
| } | |
| if (queryParams && Object.keys(queryParams).length > 0) { | |
| const qs = Object.entries(queryParams) | |
| .map(([k, v]) => `${encodeURIComponent(k)}=${encodeURIComponent(v)}`) | |
| .join('&'); | |
| url += `?${qs}`; | |
| } | |
| let code = `curl -X ${method} '${url}' \\\n`; | |
| code += ` -H 'Content-Type: application/json' \\\n`; | |
| code += ` -H 'Authorization: Bearer $FGA_BEARER_TOKEN'`; | |
| if (body) { | |
| // Escape single quotes so the shell-quoted -d body survives apostrophes in example data. | |
| const json = JSON.stringify(body, null, 2).replace(/'/g, `'\\''`); | |
| code += ` \\\n -d '${json}'`; | |
| } | |
| if (responseExample) { | |
| code += `\n\n# Response: ${responseExample}`; | |
| } | |
| return code; | |
| } |
🤖 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/Docs/SnippetViewer/ExecuteApiRequestViewer.tsx` around lines
271 - 294, The curl branch in ExecuteApiRequestViewer.tsx builds URLs and bodies
without encoding or escaping: update the path param substitution (the url
mutation that does url.split(`{${k}}`).join(v)) to insert encodeURIComponent(v)
for each path value; when building the query string (the map producing
`${k}=${v}`) use encodeURIComponent for both key and value; and make the body
safe for shell by JSON.stringify(body, null, 2) and then escaping single quotes
inside that JSON (replace each ' with the shell-safe sequence '"'"') before
wrapping it in single quotes in the -d argument so the generated curl string
(the variable code) never breaks on apostrophes.
| case SupportedLanguage.CURL: { | ||
| let url = `$FGA_API_URL${path}`; | ||
| if (pathParams) { | ||
| for (const [k, v] of Object.entries(pathParams)) { | ||
| url = url.split(`{${k}}`).join(v); | ||
| } | ||
| } | ||
| if (queryParams && Object.keys(queryParams).length > 0) { | ||
| const qs = Object.entries(queryParams) | ||
| .map(([k, v]) => `${k}=${v}`) | ||
| .join('&'); | ||
| url += `?${qs}`; | ||
| } | ||
| let code = `curl -X ${method} '${url}' \\\n`; | ||
| code += ` -H 'Content-Type: application/json' \\\n`; | ||
| code += ` -H 'Authorization: Bearer $FGA_BEARER_TOKEN'`; | ||
| if (body) { | ||
| code += ` \\\n -d '${JSON.stringify(body, null, 2)}'`; | ||
| } | ||
| return code; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
curl streaming response no-buffer flag -N documentation
💡 Result:
The -N or --no-buffer flag in curl disables output buffering, enabling real-time streaming of responses as data arrives, rather than in buffered chunks. This is particularly useful for streaming responses like Server-Sent Events (SSE), live updates, or piping to other commands without delays. From the official curl manual: "-N, --no-buffer: Disables the buffering of the output stream. In normal work situations, curl will use a standard buffered output stream that will have the effect that it will output the data in chunks, not necessarily exactly when the data arrives. Using this option will disable that buffering. Note that this is the negated option name documented. You can thus use --buffer to enforce the buffering."[1][2][3][4] Example usage for streaming: curl -N https://example.com/stream-endpoint[5][6] This flag affects stdout output and is key for progressive display or processing of long-running responses.[7][8]
Citations:
- 1: https://explainshell.com/explain?cmd=curl+-N
- 2: https://www.unix.com/man_page/osx/1/curl/
- 3: https://www.unix.com/man-page/centos/1/curl/
- 4: https://manpages.ubuntu.com/manpages/xenial/en/man1/curl.1.html
- 5: https://stackoverflow.com/questions/8362428/stream-response-from-curl-request-without-waiting-for-it-to-finish
- 6: https://stackoverflow.com/questions/77467905/saving-curl-streaming-api-output-to-a-file
- 7: https://curl.haxx.se/docs/manpage.html
- 8: https://curl.se/docs/manpage.html
🏁 Script executed:
# First, locate and read the file
fd -t f "ExecuteApiRequestViewer.tsx" | head -5Repository: openfga/openfga.dev
Length of output: 125
🏁 Script executed:
# Get file size to determine how to read it
wc -l "src/components/Docs/SnippetViewer/ExecuteApiRequestViewer.tsx"Repository: openfga/openfga.dev
Length of output: 129
🏁 Script executed:
# Read the relevant sections: lines 260-300 (non-streaming CURL), 315-330, and 460-490 (streaming CURL)
sed -n '260,300p; 315,330p; 460,490p' "src/components/Docs/SnippetViewer/ExecuteApiRequestViewer.tsx" -nRepository: openfga/openfga.dev
Length of output: 3216
🏁 Script executed:
# Verify the full context of both functions to understand scope better
sed -n '245,310p; 455,500p' "src/components/Docs/SnippetViewer/ExecuteApiRequestViewer.tsx"Repository: openfga/openfga.dev
Length of output: 3751
🏁 Script executed:
# Check for any other streaming-related CURL handling or configuration
rg -n "streaming|CURL|curl|no-buffer|-N" "src/components/Docs/SnippetViewer/ExecuteApiRequestViewer.tsx" | head -20Repository: openfga/openfga.dev
Length of output: 609
Add -N flag and fix quote escaping in streaming CURL, plus handle missing responseExample comment.
The streaming CURL output has two issues:
-
Missing
-N/--no-bufferflag. Streaming endpoints require this flag to disable output buffering, otherwise users copying the command won't observe streaming behavior in real-time. Add-Nimmediately aftercurl. -
Inconsistent responseExample handling. The non-streaming CURL case (lines 271–304) includes
if (responseExample)to emit a# Response:comment, but the streaming case omits this block entirely. This leaves theresponseExamplefield silently unused in the streaming function's public API.
Additionally, both cases need quote escaping for the JSON body (use .replace(/'/g, "'\\''") when embedding in single-quoted shell strings), consistent with best practices for shell safety.
🛠️ Proposed fix
case SupportedLanguage.CURL: {
let url = `$FGA_API_URL${path}`;
if (pathParams) {
for (const [k, v] of Object.entries(pathParams)) {
url = url.split(`{${k}}`).join(v);
}
}
if (queryParams && Object.keys(queryParams).length > 0) {
const qs = Object.entries(queryParams)
.map(([k, v]) => `${k}=${v}`)
.join('&');
url += `?${qs}`;
}
- let code = `curl -X ${method} '${url}' \\\n`;
+ let code = `curl -N -X ${method} '${url}' \\\n`;
code += ` -H 'Content-Type: application/json' \\\n`;
code += ` -H 'Authorization: Bearer $FGA_BEARER_TOKEN'`;
if (body) {
- code += ` \\\n -d '${JSON.stringify(body, null, 2)}'`;
+ const json = JSON.stringify(body, null, 2).replace(/'/g, `'\\''`);
+ code += ` \\\n -d '${json}'`;
+ }
+ if (responseExample) {
+ code += `\n\n# Response: ${responseExample}`;
}
return code;
}Consider extracting the duplicated CURL builder logic (lines 271–304 and 465–490 are nearly identical) into a helper function that accepts a streaming: boolean parameter. This would eliminate ~20 lines of duplication and ensure encoding/streaming fixes are applied consistently in one place.
🤖 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/Docs/SnippetViewer/ExecuteApiRequestViewer.tsx` around lines
465 - 485, In the SupportedLanguage.CURL branch inside
ExecuteApiRequestViewer.tsx (the CURL case building the `code` string for
method/path/url/body/responseExample), add the streaming-specific fixes: insert
`-N` (or `--no-buffer`) immediately after `curl` when building the command for
streaming endpoints, ensure any JSON body embedded in single quotes is
shell-safe by escaping single quotes (use .replace(/'/g, "'\\''") on
JSON.stringify(body, null, 2)), and emit the same `# Response:` comment using
`responseExample` as the non-streaming case so the field is not ignored;
optionally factor the duplicated CURL construction into a helper like
`buildCurlCommand({method, url, body, responseExample, streaming})` and call it
from both the non-streaming and streaming branches to keep behavior consistent.
Description
for #1254
What problem is being solved?
How is it being solved?
What changes are made to solve it?
References
Review Checklist
mainSummary by CodeRabbit
Release Notes