Skip to content

Commit ab883a2

Browse files
sshaderConvex, Inc.
authored andcommitted
Make connection state more robust to backgrounding tabs (#34622)
And also add some extra metadata around connection state. I think it's possible for a tab to be backgrounded and have the timeouts (both the interval for checking connection state + the timeout for reconnecting the WS) to be delayed. To avoid problems here, I'm requiring two consecutive disconnected connection states within `CONNECTION_STATE_CHECK_INTERVAL_MS * 2` of each other. I also realized I wanted to know a little bit more information here, like if the client ever connected successfully, whether a client reconnected successfully later after a disconnect, and also `connectionCount` (which can be a little misleading, but can be a potential indicator for a bad connection) GitOrigin-RevId: 29aa1cf348714aadd2e339a4bff01be6d9534115
1 parent 00c62da commit ab883a2

File tree

6 files changed

+173
-35
lines changed

6 files changed

+173
-35
lines changed

npm-packages/convex/src/browser/sync/client.ts

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,22 @@ export type ConnectionState = {
109109
hasInflightRequests: boolean;
110110
isWebSocketConnected: boolean;
111111
timeOfOldestInflightRequest: Date | null;
112+
/**
113+
* True if the client has ever opened a WebSocket to the "ready" state.
114+
*/
115+
hasEverConnected: boolean;
116+
/**
117+
* The number of times this client has connected to the Convex backend.
118+
*
119+
* A number of things can cause the client to reconnect -- server errors,
120+
* bad internet, auth expiring. But this number being high is an indication
121+
* that the client is having trouble keeping a stable connection.
122+
*/
123+
connectionCount: number;
124+
/**
125+
* The number of times this client has tried (and failed) to connect to the Convex backend.
126+
*/
127+
connectionRetries: number;
112128
};
113129

114130
/**
@@ -672,9 +688,13 @@ export class BaseConvexClient {
672688
* @returns The {@link ConnectionState} with the Convex backend.
673689
*/
674690
connectionState(): ConnectionState {
691+
const wsConnectionState = this.webSocketManager.connectionState();
675692
return {
676693
hasInflightRequests: this.requestManager.hasInflightRequests(),
677-
isWebSocketConnected: this.webSocketManager.socketState() === "ready",
694+
isWebSocketConnected: wsConnectionState.isConnected,
695+
hasEverConnected: wsConnectionState.hasEverConnected,
696+
connectionCount: wsConnectionState.connectionCount,
697+
connectionRetries: wsConnectionState.connectionRetries,
678698
timeOfOldestInflightRequest:
679699
this.requestManager.timeOfOldestInflightRequest(),
680700
};

npm-packages/convex/src/browser/sync/web_socket_manager.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,7 @@ export class WebSocketManager {
101101
private socket: Socket;
102102

103103
private connectionCount: number;
104+
private _hasEverConnected: boolean = false;
104105
private lastCloseReason: string | null;
105106

106107
/** Upon HTTPS/WSS failure, the first jittered backoff duration, in ms. */
@@ -197,6 +198,7 @@ export class WebSocketManager {
197198
};
198199
this.resetServerInactivityTimeout();
199200
if (this.socket.paused === "no") {
201+
this._hasEverConnected = true;
200202
this.onOpen({
201203
connectionCount: this.connectionCount,
202204
lastCloseReason: this.lastCloseReason,
@@ -498,6 +500,20 @@ export class WebSocketManager {
498500
this.connect();
499501
}
500502

503+
connectionState(): {
504+
isConnected: boolean;
505+
hasEverConnected: boolean;
506+
connectionCount: number;
507+
connectionRetries: number;
508+
} {
509+
return {
510+
isConnected: this.socket.state === "ready",
511+
hasEverConnected: this._hasEverConnected,
512+
connectionCount: this.connectionCount,
513+
connectionRetries: this.retries,
514+
};
515+
}
516+
501517
private _logVerbose(message: string) {
502518
this.logger.logVerbose(message);
503519
}

npm-packages/dashboard-common/src/lib/deploymentContext.tsx

Lines changed: 132 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
11
import { ConvexProvider, ConvexReactClient } from "convex/react";
2-
import { ConvexHttpClient } from "convex/browser";
2+
import { ConnectionState, ConvexHttpClient } from "convex/browser";
33
import {
44
createContext,
55
ReactNode,
6+
useCallback,
67
useContext,
78
useEffect,
89
useLayoutEffect,
@@ -24,6 +25,12 @@ export type DeploymentInfo = (
2425
}
2526
| { ok: false; errorCode: string; errorMessage: string }
2627
) & {
28+
addBreadcrumb: (breadcrumb: {
29+
message?: string;
30+
data?: {
31+
[key: string]: any;
32+
};
33+
}) => void;
2734
captureMessage: (msg: string) => void;
2835
captureException: (e: any) => void;
2936
reportHttpError: (
@@ -338,36 +345,61 @@ export function WaitForDeploymentApi({
338345
);
339346
}
340347

348+
const CONNECTION_STATE_CHECK_INTERVAL_MS = 2500;
349+
341350
function DeploymentWithConnectionState({
342351
deployment,
343352
children,
344353
}: {
345354
deployment: ConnectedDeployment;
346355
children: ReactNode;
347356
}) {
348-
const { isSelfHosted, captureMessage } = useContext(DeploymentInfoContext);
357+
const { isSelfHosted, captureMessage, addBreadcrumb } = useContext(
358+
DeploymentInfoContext,
359+
);
349360
const { client, deploymentUrl, deploymentName } = deployment;
350-
const [connectionState, setConnectionState] = useState<
351-
"Connected" | "Disconnected" | "LocalDeploymentMismatch" | null
352-
>(null);
353-
const [isDisconnected, setIsDisconnected] = useState(false);
354-
useEffect(() => {
355-
const checkConnection = setInterval(async () => {
356-
if (connectionState === "LocalDeploymentMismatch") {
357-
// Connection status doesn't matter since we're connected to the wrong deployment
358-
return;
359-
}
361+
const [lastObservedConnectionState, setLastObservedConnectionState] =
362+
useState<
363+
| {
364+
state: ConnectionState;
365+
time: Date;
366+
}
367+
| "LocalDeploymentMismatch"
368+
| null
369+
>(null);
370+
const [isDisconnected, setIsDisconnected] = useState<boolean | null>(null);
360371

361-
// Check WS connection status -- if we're disconnected twice in a row, treat
362-
// the deployment as disconnected.
363-
const nextConnectionState = client.connectionState();
372+
const handleConnectionStateChange = useCallback(
373+
async (
374+
state: ConnectionState,
375+
previousState: {
376+
time: Date;
377+
state: ConnectionState;
378+
} | null,
379+
): Promise<
380+
"Unknown" | "Disconnected" | "Connected" | "LocalDeploymentMismatch"
381+
> => {
382+
if (previousState === null) {
383+
return "Unknown";
384+
}
364385
if (
365-
nextConnectionState.isWebSocketConnected === false &&
366-
connectionState === "Disconnected"
386+
previousState.time.getTime() <
387+
Date.now() - CONNECTION_STATE_CHECK_INTERVAL_MS * 2
367388
) {
368-
setIsDisconnected(true);
389+
// If the previous state was observed a while ago, consider it stale (maybe the tab
390+
// got backgrounded).
391+
return "Unknown";
369392
}
370-
if (nextConnectionState.isWebSocketConnected === true) {
393+
394+
if (state.isWebSocketConnected === false) {
395+
if (previousState.state.isWebSocketConnected === false) {
396+
// we've been in state `Disconnected` twice in a row, consider the deployment
397+
// to be disconnected.
398+
return "Disconnected";
399+
}
400+
return "Unknown";
401+
}
402+
if (state.isWebSocketConnected === true) {
371403
// If this is a local deployment, check that the instance name matches what we expect.
372404
if (deploymentName.startsWith("local-")) {
373405
let instanceNameResp: Response | null = null;
@@ -381,30 +413,96 @@ function DeploymentWithConnectionState({
381413
if (instanceNameResp !== null && instanceNameResp.ok) {
382414
const instanceName = await instanceNameResp.text();
383415
if (instanceName !== deploymentName) {
384-
setConnectionState("LocalDeploymentMismatch");
385-
setIsDisconnected(true);
386-
return;
416+
return "LocalDeploymentMismatch";
387417
}
388418
}
389419
}
390-
setIsDisconnected(false);
420+
return "Connected";
421+
}
422+
return "Unknown";
423+
},
424+
[deploymentName, deploymentUrl],
425+
);
426+
427+
useEffect(() => {
428+
// Poll `.connectionState()` every 5 seconds. If we're disconnected twice in a row,
429+
// consider the deployment to be disconnected.
430+
const checkConnection = setInterval(async () => {
431+
if (lastObservedConnectionState === "LocalDeploymentMismatch") {
432+
// Connection status doesn't matter since we're connected to the wrong deployment
433+
return;
391434
}
392-
setConnectionState(
393-
nextConnectionState.isWebSocketConnected ? "Connected" : "Disconnected",
435+
// Check WS connection status -- if we're disconnected twice in a row, treat
436+
// the deployment as disconnected.
437+
const nextConnectionState = client.connectionState();
438+
const isLocalDeployment = deploymentName.startsWith("local-");
439+
const result = await handleConnectionStateChange(
440+
nextConnectionState,
441+
lastObservedConnectionState,
394442
);
395-
}, 2500);
443+
setLastObservedConnectionState({
444+
state: nextConnectionState,
445+
time: new Date(),
446+
});
447+
switch (result) {
448+
case "Disconnected":
449+
// If this is first time transitioning to disconnected, log to sentry that we've disconnected
450+
if (isDisconnected !== true) {
451+
if (!isLocalDeployment) {
452+
addBreadcrumb({
453+
message: `Cloud deployment disconnected: ${deploymentName}`,
454+
data: {
455+
hasEverConnected: nextConnectionState.hasEverConnected,
456+
connectionCount: nextConnectionState.connectionCount,
457+
connectionRetries: nextConnectionState.connectionRetries,
458+
},
459+
});
460+
// Log to sentry including the instance name when we seem to be unable to connect to a cloud deployment
461+
captureMessage(`Cloud deployment is disconnected`);
462+
}
463+
}
464+
setIsDisconnected(true);
465+
break;
466+
case "LocalDeploymentMismatch":
467+
setLastObservedConnectionState("LocalDeploymentMismatch");
468+
break;
469+
case "Unknown":
470+
setIsDisconnected(null);
471+
break;
472+
case "Connected":
473+
// If transitioning from disconnected to connected, log to sentry that we've reconnected
474+
if (isDisconnected === true) {
475+
if (!isLocalDeployment) {
476+
addBreadcrumb({
477+
message: `Cloud deployment reconnected: ${deploymentName}`,
478+
});
479+
// Log to sentry including the instance name when we seem to be unable to connect to a cloud deployment
480+
captureMessage(`Cloud deployment has reconnected`);
481+
}
482+
}
483+
setIsDisconnected(false);
484+
break;
485+
default: {
486+
const _exhaustiveCheck: never = result;
487+
throw new Error(`Unknown connection state: ${result}`);
488+
}
489+
}
490+
}, CONNECTION_STATE_CHECK_INTERVAL_MS);
396491
return () => clearInterval(checkConnection);
397-
});
398-
useEffect(() => {
399-
if (isDisconnected && !deploymentName.startsWith("local-")) {
400-
// Log to sentry including the instance name when we seem to be unable to connect to a cloud deployment
401-
captureMessage(`Cloud deployment is disconnected: ${deploymentName}`);
402-
}
403-
}, [isDisconnected, deploymentName, captureMessage]);
492+
}, [
493+
lastObservedConnectionState,
494+
deploymentName,
495+
deploymentUrl,
496+
client,
497+
addBreadcrumb,
498+
captureMessage,
499+
handleConnectionStateChange,
500+
isDisconnected,
501+
]);
404502
const value = useMemo(
405503
() => ({
406504
deployment,
407-
isDisconnected,
505+
isDisconnected: isDisconnected === true,
408506
}),
409507
[deployment, isDisconnected],
410508
);

npm-packages/dashboard-common/src/lib/mockDeploymentInfo.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ export const mockDeploymentInfo: DeploymentInfo = {
55
reportHttpError: () => {},
66
captureException: () => {},
77
captureMessage: () => {},
8+
addBreadcrumb: () => {},
89
deploymentUrl: process.env.NEXT_PUBLIC_DEPLOYMENT_URL!,
910
adminKey: process.env.NEXT_PUBLIC_ADMIN_KEY!,
1011
useCurrentTeam: () => ({

npm-packages/dashboard-self-hosted/src/pages/_app.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,7 @@ export default App;
9999

100100
const deploymentInfo: Omit<DeploymentInfo, "deploymentUrl" | "adminKey"> = {
101101
ok: true,
102+
addBreadcrumb: console.error,
102103
captureMessage: console.error,
103104
captureException: console.error,
104105
reportHttpError: (

npm-packages/dashboard/src/providers/DeploymentInfoProvider.tsx

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { useRouter } from "next/router";
33
import {
44
captureException,
55
captureMessage,
6+
addBreadcrumb,
67
ErrorBoundary,
78
} from "@sentry/nextjs";
89
import { reportHttpError } from "hooks/fetching";
@@ -59,6 +60,7 @@ export function DeploymentInfoProvider({
5960
);
6061
setDeploymentInfo({
6162
...info,
63+
addBreadcrumb,
6264
captureMessage,
6365
captureException,
6466
reportHttpError,

0 commit comments

Comments
 (0)