fix: added visual loading spinner and success/error toast when restoring default langflow flows#1931
fix: added visual loading spinner and success/error toast when restoring default langflow flows#1931Vchen7629 wants to merge 2 commits into
Conversation
Walkthrough
ChangesRestore flow loading state and toast feedback
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/app/settings/_components/ingest-settings-section.tsx (1)
235-239:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAvoid unconditional JSON parsing on successful restore response.
At Line 237, parsing JSON for every successful response can throw on empty success responses and incorrectly trigger the error toast path.
Suggested fix
fetch("/api/reset-flow/ingest", { method: "POST" }) .then((res) => { - if (res.ok) return res.json(); - throw new Error(`HTTP ${res.status}: ${res.statusText}`); + if (!res.ok) throw new Error(`HTTP ${res.status}: ${res.statusText}`); + return res; + }) + .then(async (res) => { + const contentType = res.headers.get("content-type") || ""; + if (contentType.includes("application/json")) { + await res.json(); + } + return; })🤖 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 `@frontend/app/settings/_components/ingest-settings-section.tsx` around lines 235 - 239, The fetch call to "/api/reset-flow/ingest" with POST method unconditionally calls res.json() on successful responses, which will throw an error for empty response bodies and incorrectly trigger error handling even though the request succeeded. Modify the .then() handler to check if the response has content (by checking the Content-Length header or response size) before attempting to parse JSON, or conditionally return the parsed JSON only when the body is not empty, otherwise return a default success value to avoid parsing errors on empty successful responses.
🤖 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 `@frontend/app/settings/_components/agent-settings-section.tsx`:
- Around line 186-193: The handleRestoreRetrievalFlow function unconditionally
calls res.json() for all successful responses, but the endpoint may return a
successful response with an empty body, which causes JSON parsing to fail.
Modify the .then() handler to check if the response has content before
attempting to parse JSON. You can do this by checking the Content-Length header
or the response status, or by using res.text() first and only parsing as JSON if
there is actual content in the response body. This will prevent false error
toasts when the endpoint returns a successful empty response.
---
Outside diff comments:
In `@frontend/app/settings/_components/ingest-settings-section.tsx`:
- Around line 235-239: The fetch call to "/api/reset-flow/ingest" with POST
method unconditionally calls res.json() on successful responses, which will
throw an error for empty response bodies and incorrectly trigger error handling
even though the request succeeded. Modify the .then() handler to check if the
response has content (by checking the Content-Length header or response size)
before attempting to parse JSON, or conditionally return the parsed JSON only
when the body is not empty, otherwise return a default success value to avoid
parsing errors on empty successful responses.
🪄 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: 98017bb7-5cf9-4283-9064-e8e3decad1c7
📒 Files selected for processing (3)
frontend/app/settings/_components/agent-settings-section.tsxfrontend/app/settings/_components/ingest-settings-section.tsxfrontend/components/confirmation-dialog.tsx
| const handleRestoreRetrievalFlow = (closeDialog: () => void) => { | ||
| setIsRestoringFlow(true); | ||
|
|
||
| fetch("/api/reset-flow/retrieval", { method: "POST" }) | ||
| .then((res) => { | ||
| if (res.ok) return res.json(); | ||
| throw new Error(`HTTP ${res.status}: ${res.statusText}`); | ||
| }) |
There was a problem hiding this comment.
Handle successful empty responses without forcing JSON parse.
At Line 191, res.json() is called for every successful response. If the endpoint returns success with no JSON body, this throws and shows a false error toast.
Suggested fix
fetch("/api/reset-flow/retrieval", { method: "POST" })
.then((res) => {
- if (res.ok) return res.json();
+ if (!res.ok) throw new Error(`HTTP ${res.status}: ${res.statusText}`);
+ return res;
+ })
+ .then(async (res) => {
+ const contentType = res.headers.get("content-type") || "";
+ if (contentType.includes("application/json")) {
+ await res.json();
+ }
+ return;
- throw new Error(`HTTP ${res.status}: ${res.statusText}`);
})📝 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.
| const handleRestoreRetrievalFlow = (closeDialog: () => void) => { | |
| setIsRestoringFlow(true); | |
| fetch("/api/reset-flow/retrieval", { method: "POST" }) | |
| .then((res) => { | |
| if (res.ok) return res.json(); | |
| throw new Error(`HTTP ${res.status}: ${res.statusText}`); | |
| }) | |
| const handleRestoreRetrievalFlow = (closeDialog: () => void) => { | |
| setIsRestoringFlow(true); | |
| fetch("/api/reset-flow/retrieval", { method: "POST" }) | |
| .then((res) => { | |
| if (!res.ok) throw new Error(`HTTP ${res.status}: ${res.statusText}`); | |
| return res; | |
| }) | |
| .then(async (res) => { | |
| const contentType = res.headers.get("content-type") || ""; | |
| if (contentType.includes("application/json")) { | |
| await res.json(); | |
| } | |
| return; | |
| }) |
🤖 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 `@frontend/app/settings/_components/agent-settings-section.tsx` around lines
186 - 193, The handleRestoreRetrievalFlow function unconditionally calls
res.json() for all successful responses, but the endpoint may return a
successful response with an empty body, which causes JSON parsing to fail.
Modify the .then() handler to check if the response has content before
attempting to parse JSON. You can do this by checking the Content-Length header
or the response status, or by using res.text() first and only parsing as JSON if
there is actual content in the response body. This will prevent false error
toasts when the endpoint returns a successful empty response.
richardz403
left a comment
There was a problem hiding this comment.
Looks good to me.
Ran locally, tested that visual loading spinner does work correctly.
Summary
Fixes #1286 , Adds loading state feedback for the restore retrieval flow functionality to improve user experience.
Demo (Before)
screen-capture.4.webm
Demo (After)
screen-capture.2.webm
Changes
agent-settings-section.tsxandingest-settings-section.tsxhandleRestoreRetrieval()to display success/error toast notificationsconfirmation-dialog.tsxwith optionalisLoadingpropTesting
Summary by CodeRabbit
Release Notes