diff --git a/clients/web/src/hooks/useConnectionLifecycle.test.tsx b/clients/web/src/hooks/useConnectionLifecycle.test.tsx index 3839cfc1..e09a44b9 100644 --- a/clients/web/src/hooks/useConnectionLifecycle.test.tsx +++ b/clients/web/src/hooks/useConnectionLifecycle.test.tsx @@ -101,13 +101,20 @@ interface HarnessProps { activeServerId?: string; connectionStatus?: ConnectionStatus; /** - * The live client. `undefined` means "whatever the last - * `setupClientForServer` built", which is what App.tsx's `inspectorClient` - * state settles to a render later. + * The live client, i.e. what App.tsx holds in `inspectorClient`. There is no + * implicit "whatever was last constructed" fallback — the hook publishes + * through `setInspectorClient`, which is a spy here, so a test that wants a + * live session rerenders with the client it read back out of that spy (see + * `lastClient`), the same render-later settle App.tsx has. */ client?: InspectorClient | null; clientConfig?: ClientConfig; sandboxUrl?: string; + /** + * The `/api/config` gate a connect awaits. Defaults to already-settled; + * supply a pending promise to hold a connect at that gate. + */ + configSettled?: Promise; deepLink?: DeepLink; reAuthBanner?: { serverId: string; @@ -177,7 +184,9 @@ function harness(initial: HarnessProps = {}): Harness { // dependency the real one never has. const setupClientForServerRef = useRef(null); const initialConfigSettledRef = useRef<{ promise: Promise }>(null); - initialConfigSettledRef.current ??= { promise: Promise.resolve() }; + initialConfigSettledRef.current ??= { + promise: p.configSettled ?? Promise.resolve(), + }; const sessionReset: SessionResetSurface = { clearResultPanels: s.clearResultPanels, resetTabUiState: s.resetTabUiState, @@ -404,6 +413,43 @@ describe("useConnectionLifecycle", () => { expect(h.spies.newAppElicitationSession).not.toHaveBeenCalled(); }); + it("waits for the config gate, then reads the sandbox URL as of then", async () => { + // The whole reason `onToggleConnection` awaits the gate and + // `setupClientForServer` reads `sandboxUrlRef` rather than a captured + // value: on the load this matters for, the connect starts before + // `/api/config` has answered. Guessing "no sandbox" there would strand + // the session on the native elicitation form despite having one. + let settle!: () => void; + const gate = new Promise((resolve) => (settle = resolve)); + const props: HarnessProps = { + servers: [entry("a")], + configSettled: gate, + sandboxUrl: undefined, + }; + const h = harness(props); + + let toggled: Promise; + act(() => { + toggled = h.api().onToggleConnection("a"); + }); + // Held at the gate — nothing constructed, nothing connected. + expect(h.spies.setInspectorClient).not.toHaveBeenCalled(); + expect(connectSpy).not.toHaveBeenCalled(); + + // `/api/config` answers: there IS a sandbox after all. + h.rerender({ ...props, sandboxUrl: "http://localhost:6275/sandbox" }); + + await act(async () => { + settle(); + await toggled; + }); + + expect(connectSpy).toHaveBeenCalledTimes(1); + // Built from the URL as of the release, not the `undefined` in scope when + // the toggle was called. + expect(h.spies.newAppElicitationSession).toHaveBeenCalled(); + }); + it("carries the OAuth session id onto both the client and its stores", () => { const h = harness({ servers: [entry("a")] }); @@ -1190,6 +1236,52 @@ describe("useConnectionLifecycle", () => { ); }); + it("records a construction throw instead of leaving it unhandled", async () => { + // Client construction runs ahead of the toggle's try/catch, so a throw + // there escapes `onToggleConnection`. Both banner call sites discard the + // promise, so without `retryConnect` this is an unhandled rejection with + // the banner already dismissed and nothing left to explain it. + const h = harness({ + servers: [entry("a"), entry("b")], + activeServerId: "a", + connectionStatus: "connected", + reAuthBanner: { serverId: "b", message: "lapsed" }, + }); + h.spies.destroyStores.mockImplementationOnce(() => { + throw new Error("teardown wedged"); + }); + + await act(async () => { + h.api().onReauthenticateFromBanner(); + }); + + await waitFor(() => + expect(h.api().connectErrorMessage).toBe("teardown wedged"), + ); + }); + + it("records a construction throw on the lost-state retry too", async () => { + const h = harness({ + servers: [entry("a")], + reAuthBanner: { + serverId: "a", + message: "lost", + kind: "lost_authorization_state", + }, + }); + h.spies.destroyStores.mockImplementationOnce(() => { + throw "teardown wedged"; + }); + + await act(async () => { + h.api().onReauthenticateFromBanner(); + }); + + await waitFor(() => + expect(h.api().connectErrorMessage).toBe("teardown wedged"), + ); + }); + it("toggles the connection when the banner's server is not the live one", async () => { const h = harness({ servers: [entry("a"), entry("b")], diff --git a/clients/web/src/hooks/useConnectionLifecycle.ts b/clients/web/src/hooks/useConnectionLifecycle.ts index 44e2aaed..934b73e6 100644 --- a/clients/web/src/hooks/useConnectionLifecycle.ts +++ b/clients/web/src/hooks/useConnectionLifecycle.ts @@ -862,6 +862,30 @@ export function useConnectionLifecycle({ recordConnectError, ]); + /** + * The banner's connect, terminated. `onToggleConnection` handles every + * failure the *handshake* can produce, but client construction runs ahead of + * its try/catch — so a throw there (a wedged store teardown, a config the + * `InspectorClient` constructor rejects) escapes it. Both banner call sites + * discard the promise, which would make that an unhandled rejection with the + * banner already dismissed and nothing left to explain it. + * + * Recorded on `connectErrorMessage` rather than toasted, matching the + * deep-link phases' own catch: the toggle already toasts everything it + * handles, so a toast here would be new user-visible behavior in what is + * otherwise an inert move. + */ + const retryConnect = useCallback( + async (serverId: string) => { + try { + await onToggleConnection(serverId); + } catch (err) { + recordConnectError(err instanceof Error ? err.message : String(err)); + } + }, + [onToggleConnection, recordConnectError], + ); + const onReauthenticateFromBanner = useCallback(() => { if (!reAuthBanner) return; const serverId = reAuthBanner.serverId; @@ -901,7 +925,7 @@ export function useConnectionLifecycle({ return; } } - await onToggleConnection(serverId); + await retryConnect(serverId); })(); return; } @@ -944,16 +968,16 @@ export function useConnectionLifecycle({ return; } - void onToggleConnection(serverId); + void retryConnect(serverId); }, [ sessionRef, reAuthBanner, + retryConnect, activeServerId, connectionStatus, inspectorClient, servers, prepareOAuthRedirect, - onToggleConnection, webOAuthStorage, setReAuthBanner, ]);