fix: reset dialog state and abort in-flight test on cancel (#2048)#2069
fix: reset dialog state and abort in-flight test on cancel (#2048)#2069hunterxtang wants to merge 1 commit into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughConnector settings dialogs now unmount when closed. Azure Blob connection tests cancel prior or unmounted requests, suppress abort errors, and preserve loading state for the currently active request. ChangesConnector dialog reliability
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
10ceb00 to
4a2d325
Compare
|
Superseded by #2072 — re-landed on an in-repo branch so the secret-dependent checks (build-images, e2e) can run. |
Fixes #2048 on main.
Root cause
Connector settings dialogs (Azure Blob, S3, IBM COS) are rendered
unconditionally in
connector-cards.tsxwith a stablekey, so thecomponent never unmounts when the user closes the dialog. All
useStateanduseFormstate — the connection-string field value,the
formError"Test the connection first…" message, and theisFetchingContainersloading flag — survive Cancel and reappear onthe next open.
Additionally, the Azure Blob test-connection handler called
fetch("/api/connectors/azure_blob/test")with noAbortController.Clicking Cancel while a test was in flight left
isFetchingContainersstuck at
trueuntil the underlying TCP connection timed out, keepingthe button locked on "Connecting…" on reopen.
Fix
1 —
connector-cards.tsx: unmount on closeRender each
SettingsDialogonly whileopenDialog === descriptor.connectorType;return
nullotherwise. The component unmounts on close, React destroys allinternal state, and every field and error message is fresh on the next open.
This also resolves the same latent stale-state bug in the S3 and IBM COS
dialogs at no extra cost, since they go through the same rendering loop.
2 —
azure-blob/settings-dialog.tsx: AbortController on the test fetchuseRef<AbortController | null>tracks the currently in-flight test request.handleTestConnection, any prior call is aborted before a newcontroller is created — cancels a stuck request if the user clicks the button
twice.
signal: controller.signalis passed tofetch;AbortErroris caught andswallowed silently (no error state set to the user).
useEffectcleanup aborts on unmount, so closing the dialog mid-requestimmediately cancels the network call rather than letting it run to TCP timeout.
finallyblock is guarded withtestAbortRef.current === controllersothat an aborted call's
finallydoes not clearisFetchingContainerswhile asuperseding request is still in flight (prevents spinner flicker).
Known tradeoff
Unmount-on-close skips the Radix exit animation — the dialog disappears
immediately rather than fading out. Intentional; correctness takes priority over
the transition.
Deferred follow-up
S3 (
s3-settings-dialog.tsx) and IBM COS (ibm-cos/settings-dialog.tsx) bothuse a
mutateAsync+ rawfetchsequence in their test paths. The rawfetchportion is equally abortable, but adding
AbortControllerto both is deferredto keep this PR scoped to #2048.
Verification
biome checkandtsc --noEmiton the two changed files: no new warnings orerrors vs. the baseline on
main.container build of this branch (Azurite down): connection string, stale error
message, and stuck loading state all confirmed fixed after Cancel + reopen.
Summary by CodeRabbit