fix(frontend): stop stale flows-list refetch from bouncing New Flow back to the list - #14288
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughRefresh-flow queries now forward abort signals and suppress cancellation notifications. FlowPage fetches missing route flows and applies them to the canvas before redirecting when retrieval or application fails. ChangesRefresh flow cancellation
Missing flow loading
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant ReactQuery
participant getFlowsFn
participant ApiClient
participant FlowsStore
ReactQuery->>getFlowsFn: invoke with AbortSignal
getFlowsFn->>ApiClient: request flows with signal
getFlowsFn->>ApiClient: request components with signal
ApiClient-->>getFlowsFn: flows or cancellation error
getFlowsFn->>FlowsStore: setFlows on success
Suggested reviewers: 🚥 Pre-merge checks | ✅ 8 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (8 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 |
✅ Test Coverage AdvisorNo source changes detected without accompanying tests. Thanks for keeping coverage up! 🎉
|
github-actions
Bot
added
bug
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/frontend/src/pages/FlowPage/index.tsx`:
- Around line 185-190: Update the async effect containing getFlow and
applyFlowToCanvas with an effect-local cancellation guard that is set during
cleanup when id changes or the component unmounts. Check the guard before
applying the fetched flow and before calling navigate("/all"), preserving the
existing behavior for the current active request.
🪄 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: 815314d3-c057-4614-b7c3-0bf7b81546c7
📒 Files selected for processing (3)
src/frontend/src/controllers/API/queries/flows/__tests__/use-get-refresh-flows-query.test.tssrc/frontend/src/controllers/API/queries/flows/use-get-refresh-flows-query.tssrc/frontend/src/pages/FlowPage/index.tsx
| try { | ||
| const flow = await getFlow({ id: id! }); | ||
| applyFlowToCanvas(flow); | ||
| } catch { | ||
| navigate("/all"); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Guard the async route check against stale effects.
Lines 185–190 can resolve after id changes or the component unmounts. The old request may then apply the previous route’s flow or call navigate("/all") after a newer route has loaded. Add an effect-local cancellation guard (or abort the request) before applying the flow and before redirecting.
Proposed fix
useEffect(() => {
+ let cancelled = false;
+
const awaitgetTypes = async () => {
...
try {
const flow = await getFlow({ id: id! });
+ if (cancelled) return;
applyFlowToCanvas(flow);
} catch {
+ if (cancelled) return;
navigate("/all");
}
return;
...
};
awaitgetTypes();
+
+ return () => {
+ cancelled = true;
+ };
}, [id, flows, currentFlowId, types]);📝 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.
| try { | |
| const flow = await getFlow({ id: id! }); | |
| applyFlowToCanvas(flow); | |
| } catch { | |
| navigate("/all"); | |
| } | |
| useEffect(() => { | |
| let cancelled = false; | |
| const awaitgetTypes = async () => { | |
| ... | |
| try { | |
| const flow = await getFlow({ id: id! }); | |
| if (cancelled) return; | |
| applyFlowToCanvas(flow); | |
| } catch { | |
| if (cancelled) return; | |
| navigate("/all"); | |
| } | |
| return; | |
| ... | |
| }; | |
| awaitgetTypes(); | |
| return () => { | |
| cancelled = true; | |
| }; | |
| }, [id, flows, currentFlowId, types]); |
🤖 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/frontend/src/pages/FlowPage/index.tsx` around lines 185 - 190, Update the
async effect containing getFlow and applyFlowToCanvas with an effect-local
cancellation guard that is set during cleanup when id changes or the component
unmounts. Check the guard before applying the fetched flow and before calling
navigate("/all"), preserving the existing behavior for the current active
request.
Frontend Unit Test Coverage ReportCoverage Summary
Unit Test Results
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## release-1.11.2 #14288 +/- ##
==================================================
- Coverage 61.23% 56.85% -4.39%
==================================================
Files 2450 2318 -132
Lines 239484 235326 -4158
Branches 36256 34840 -1416
==================================================
- Hits 146641 133785 -12856
- Misses 91071 99769 +8698
Partials 1772 1772
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
github-actions
Bot
added
bug
…ack to the list Clicking New Flow right after navigating back to the flows list could silently do nothing: the flow was created and the app navigated to /flow/<id>, but an in-flight header flows refetch - snapshotted BEFORE the create - landed afterwards and rewrote the flows store without the new flow, so FlowPage's existence guard bounced straight back to /all with no overlay, no toast and no console error (and an orphan flow left behind). Two layers: - use-get-refresh-flows-query now forwards the query's AbortSignal into both HTTP calls, so a superseded fetch (create fires refetchQueries) or an unmounted observer aborts for real and its setFlows side effect never runs; cancellations no longer surface the could-not-load-flows toast. - FlowPage's guard no longer treats the flows store as authoritative when the route id is missing: it confirms with GET /flows/<id> and only redirects to /all when the server does not know the flow either. Repro (deterministic, Playwright against langflow-nightly): hold the back-nav header_flows response until after the POST + navigate while the single-flow GET is in flight -> before: /flows -> /flow/<id> -> /all bounce, no welcome overlay; after: lands on the canvas with the welcome overlay. Found by the E2E regression suite (oriontech-me/langflow-e2e#966, filed as LE-2019); affects 1.10.1+ via #12575.
662b84d to
be8180a
Compare
github-actions
Bot
added
bug
Review notes — non-blockingVerdict: LGTM. Root cause is real, fix is minimal and layered. A few small items: 🐛 Suggestions
🧪 Test gap
📝 Notes
|
github-actions
Bot
added
bug
|
Thanks @viktoravelino — addressed these in
Validation: Biome clean; 75 related suites / 1,005 tests passed, then the 10 focused tests were rerun after the concurrent release-branch merge. |
github-actions
Bot
added
bug
2 participants
Summary
Fixes LE-2019 (found by the E2E regression suite, oriontech-me/langflow-e2e#966): clicking New Flow immediately after returning to the flows list via the header back chevron silently does nothing — no navigation, no welcome overlay, no templates modal, no toast, no console error — and leaves an orphan empty flow behind. Affects 1.10.1+ (create-then-navigate introduced by #12575); the failing clicks correlate exactly with "zero flow cards painted yet".
Root cause (reproduced deterministically)
/flowsremounts the app-headerFlowMenu, which refetchesGET /api/v1/flows/?get_all=true&header_flows=true(staleTime0). Its response is a pre-creation snapshot.useStartNewFlow:POST /api/v1/flows/→ 201 →setFlows([created, …])→navigate("/flow/<id>").FlowPagemounts and starts loading the flow;currentFlowIdstays""until the canvas GET resolves.usePostAddFlow.onSettled→refetchQueries), but the queryFn'sapi.getcarries noAbortSignal, so the "cancelled" fetch keeps running and itssetFlows(stale)side effect rewrites the store without the new flow.FlowPage's existence guard (flows.find(id)miss whilecurrentFlowId === "") silentlynavigate("/all")— the user is bounced back to the list with nothing to show. Captured URL trail:/flows → /flow/<id> → /all.The "cards: 0" correlation from the bug report is exactly this window: cards not painted ⇔ the back-nav queries are still in flight ⇔ a stale response is guaranteed to land after the navigation.
Fix (two independent layers)
use-get-refresh-flows-query.ts— forward the query'sAbortSignalinto bothapi.getcalls, so a superseded fetch (or one whose observers unmounted) aborts for real and never reachessetFlows. Cancellations are also excluded from the "could not load flows" error toast (axios.isCancel).FlowPage/index.tsx— when the route's flow id is missing from the (mutable, transiently stale) flows store, confirm withGET /flows/<id>before giving up; only redirect to/allwhen the server doesn't know the flow either. A genuinely nonexistent id still redirects exactly as before.Deliberately not included: the related welcome-overlay placeholder-cleanup defect (blank-check reads
datafromheader_flowspayloads, which is always null) noted in the E2E team's evidence log — it is tracked separately.Test plan
use-get-refresh-flows-query.test.ts(5 tests): signal forwarded to both GETs; aborted fetch never writes the store and never toasts (first and second call); 500 still toasts; 403 stays silent.controllers/API/queries/flows,pages/FlowPage,hooks/flows,flowBuilderWelcome,createNewFlow.tsc --noEmit: no new errors in touched files; Biome clean.langflowai/langflow-nightly:latest,1.12.0.dev7,LANGFLOW_AUTO_LOGIN=true,LANGFLOW_WORKERS=1): hold the back-navheader_flowsresponse (server snapshot taken pre-create) until after POST+navigate while the single-flow GET is in flight./flows → /flow/<id> → /all, no overlay/modal — the reported bug, reproduced on demand./flow/<id>with the welcome overlay rendered, same hostile timing./flow/00000000-…) still redirects to/all.Refs oriontech-me/langflow-e2e#966
Summary by CodeRabbit
Bug Fixes
Tests