Draft fix for console data spinner#36613
Draft
frankmcsherry wants to merge 1 commit into
Draft
Conversation
leedqin
added a commit
that referenced
this pull request
May 19, 2026
This fixes [CNS-77](https://linear.app/materializeinc/issue/CNS-77/data-explorer-page-not-loading) where the data explorer spinner keeps spinning for a while. Reported by @sjwiesman and @frankmcsherry , I have only been able to repro this in Safari. Iterated in this draft fix PR: #36613 , there is a race condition in the websocket hook useEffects that initiates the Websocket connection and the second effect kills the `CONNECTING` socket. I refactored to route the `useAutomaticallyConnectWebsocket` reconnect signals in the Websocket Connection Manager and added a guard to serialize the overlapping triggers. Fixed another bug that happens when a user navigates away from Data Explorer page and comes back, it reloads the page. Coming back to Data Explorer, opens a new Websocket connection and reset the state that clobbered the existing snapshot. Fixed the hook `useGlobalUpsertSubscribe` to hold the cached snapshot until new manager has the data. ### Verification - Added a test for the race condition Before: https://github.com/user-attachments/assets/58c2b834-f76d-4158-b4f2-ef330172bb64 - Videos of the fix here https://github.com/user-attachments/assets/8c12d53f-6d0b-48c1-8ae4-b5a433ba2ee9
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Claude and I and @leedqin iterated to repro and potentially address an issue where the Console's data pane produced a spinner than ran forever. Claude's take is a race in websocket setup, in which the two reasons a connection could be started both fire, and race, and trip over each other.
Caveat: all logging retain, by request for ease of understanding, but we should remove before merging anything (the purported fix is one line).
Claude authored summary (horrid line breaks and all):
Summary
Fixes an intermittent indefinite spinner on the Data pane (object
explorer).
The hang happens when the user navigates to the object explorer
after the
environment is already healthy.
useAutomaticallyConnectSockethastwo
effects that both initiate a websocket connect on first mount:
WebsocketConnectionManager; itsconstructor
synchronously runs
handleEnvironmentChange→resumeConnection→target.reconnect()and starts socket #1.previousRequest === undefinedand callssubscribe.connect(request, …), whichdisconnect()s socket #1while
it's still in
CONNECTINGstate and opens socket #2.The browser logs
WebSocket is closed before the connection is establishedfor socket #1. Usually socket #2 opens fine; intermittently the
browser
appears to leave it stuck in
CONNECTINGand neitheropen,close,nor
errorever fires — soSubscribeManagernever seesonOpen/
onReadyForQuery, the SUBSCRIBE is never sent,snapshotCompletestays
false, andObjectExplorerkeeps rendering<LoadingContainer />.allObjects(mounted at app boot inAppInitializer) almostalways
sidesteps this — the env isn't
enabledyet when its effects run,so
effect 1's connect path is a no-op and only effect 2 connects.
allNamespaces(mounted on entry to/regions/.../objects)reliably hits
the race because the env is already healthy by then.
Changes
useAutomaticallyConnectSocket.ts: the request-change effectnow
skips when
previousRequest === undefined(first mount). Themanager
constructor already kicks off the initial connect using the
sqlRequestcaptured bySubscribeManager's constructor, sothis
effect's job is only to handle subsequent request changes.
useSubscribe.ts:useState(new SubscribeManager(...))→useState(() => new SubscribeManager(...))in all three hookvariants.
The non-lazy form allocated a throwaway
SubscribeManager(andinner
MaterializeWebsocket) on every render. Unrelated to the hangbut
surfaced while diagnosing.
Test plan
on
first visit (previously reproduced ~50% of the time).
WebSocket is closed before the connection is established.Tweak the test-plan numbers to whatever you actually observed.