Skip to content

Commit

Permalink
[lib] Pass isSocketConnected instead of connectionStatus to callServe…
Browse files Browse the repository at this point in the history
…rEndpoint

Summary:
When `connectionStatus` cycles between `disconnected` and `connecting`, it causes `callServerEndpoint` to get regenerated, which results in every single action being regenerated as well.

However, in `callServerEndpoint` we actually only care if `connectionStatus` is `connected` or not. We can avoid all of these actions being regenerated by passing in some more specific information to `callServerEndpoint`.

This gets us closer to resolving [ENG-3612](https://linear.app/comm/issue/ENG-3612/[native]-getting-huge-number-of-unhandled-promise-rejection-when). Some more details in [this Linear comment](https://linear.app/comm/issue/ENG-3612/[native]-getting-huge-number-of-unhandled-promise-rejection-when#comment-9701ab64).

However, this only solves the issue for the old-style `useServerCall`. `useKeyserverCall` is still broken because it has a harder task of caching the whole list of `KeyserverInfo`s. The following diffs will resolve that.

Test Plan:
Before this stack, I was able to reproduce [ENG-3612](https://linear.app/comm/issue/ENG-3612/[native]-getting-huge-number-of-unhandled-promise-rejection-when) by going to the `ThreadSettings` screen in `native` while my local `keyserver` was down. After this stack, the issue no longer repros.

I also compiled a release build of the iOS app to my phone to confirm that there were no regressions in TTI, or the time it takes to open a `MessageList` and go back to the `ChatThreadList`.

Reviewers: inka, rohan

Reviewed By: rohan

Subscribers: tomek

Differential Revision: https://phab.comm.dev/D10464
  • Loading branch information
Ashoat committed Dec 29, 2023
1 parent 6ca82c0 commit cf26c80
Show file tree
Hide file tree
Showing 4 changed files with 24 additions and 25 deletions.
12 changes: 6 additions & 6 deletions lib/selectors/server-calls.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,18 +12,15 @@ import {
} from './keyserver-selectors.js';
import type { PlatformDetails } from '../types/device-types.js';
import type { AppState } from '../types/redux-types.js';
import type {
ConnectionInfo,
ConnectionStatus,
} from '../types/socket-types.js';
import type { ConnectionInfo } from '../types/socket-types.js';
import { type CurrentUserInfo } from '../types/user-types.js';

export type ServerCallState = {
+cookie: ?string,
+urlPrefix: ?string,
+sessionID: ?string,
+currentUserInfo: ?CurrentUserInfo,
+connectionStatus: ?ConnectionStatus,
+isSocketConnected: ?boolean,
+lastCommunicatedPlatformDetails: ?PlatformDetails,
};

Expand All @@ -49,7 +46,10 @@ const baseServerCallStateSelector: (
urlPrefix,
sessionID,
currentUserInfo,
connectionStatus: connectionInfo?.status,
isSocketConnected:
connectionInfo?.status !== undefined
? connectionInfo?.status === 'connected'
: undefined,
lastCommunicatedPlatformDetails,
}),
);
Expand Down
23 changes: 12 additions & 11 deletions lib/utils/action-utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ import type {
ClientSessionChange,
PreRequestUserState,
} from '../types/session-types.js';
import type { ConnectionStatus } from '../types/socket-types.js';
import type { CurrentUserInfo } from '../types/user-types.js';

function extractKeyserverIDFromID(id: string): string {
Expand Down Expand Up @@ -232,7 +231,7 @@ async function fetchNewCookieFromNativeCredentials(
() => new Promise(r => r(null)),
urlPrefix,
null,
'disconnected',
false,
null,
null,
endpoint,
Expand Down Expand Up @@ -309,7 +308,7 @@ function bindCookieAndUtilsIntoCallServerEndpoint(
urlPrefix,
sessionID,
currentUserInfo,
connectionStatus,
isSocketConnected,
lastCommunicatedPlatformDetails,
keyserverID,
} = params;
Expand Down Expand Up @@ -412,7 +411,7 @@ function bindCookieAndUtilsIntoCallServerEndpoint(
cookieInvalidationRecovery,
urlPrefix,
sessionID,
connectionStatus,
isSocketConnected,
lastCommunicatedPlatformDetails,
socketAPIHandler,
endpoint,
Expand All @@ -432,7 +431,7 @@ export type BindServerCallsParams = {
+urlPrefix: string,
+sessionID: ?string,
+currentUserInfo: ?CurrentUserInfo,
+connectionStatus: ConnectionStatus,
+isSocketConnected: boolean,
+lastCommunicatedPlatformDetails: ?PlatformDetails,
+keyserverID: string,
};
Expand All @@ -455,7 +454,7 @@ const baseCreateBoundServerCallsSelector = <F>(
(state: BindServerCallsParams) => state.urlPrefix,
(state: BindServerCallsParams) => state.sessionID,
(state: BindServerCallsParams) => state.currentUserInfo,
(state: BindServerCallsParams) => state.connectionStatus,
(state: BindServerCallsParams) => state.isSocketConnected,
(state: BindServerCallsParams) => state.lastCommunicatedPlatformDetails,
(state: BindServerCallsParams) => state.keyserverID,
(
Expand All @@ -464,7 +463,7 @@ const baseCreateBoundServerCallsSelector = <F>(
urlPrefix: string,
sessionID: ?string,
currentUserInfo: ?CurrentUserInfo,
connectionStatus: ConnectionStatus,
isSocketConnected: boolean,
lastCommunicatedPlatformDetails: ?PlatformDetails,
keyserverID: string,
) => {
Expand All @@ -474,7 +473,7 @@ const baseCreateBoundServerCallsSelector = <F>(
urlPrefix,
sessionID,
currentUserInfo,
connectionStatus,
isSocketConnected,
lastCommunicatedPlatformDetails,
keyserverID,
});
Expand All @@ -497,16 +496,18 @@ function useServerCall<F>(
serverCallStateSelector(ashoatKeyserverID),
);
return React.useMemo(() => {
const { urlPrefix, connectionStatus } = serverCallState;
const { urlPrefix, isSocketConnected } = serverCallState;
invariant(
!!urlPrefix && !!connectionStatus,
!!urlPrefix &&
isSocketConnected !== undefined &&
isSocketConnected !== null,
'keyserver missing from keyserverStore',
);

return createBoundServerCallsSelector(serverCall)({
...serverCallState,
urlPrefix,
connectionStatus,
isSocketConnected,
dispatch,
...paramOverride,
keyserverID: ashoatKeyserverID,
Expand Down
5 changes: 2 additions & 3 deletions lib/utils/call-server-endpoint.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ import type {
ServerSessionChange,
ClientSessionChange,
} from '../types/session-types.js';
import type { ConnectionStatus } from '../types/socket-types';
import type { CurrentUserInfo } from '../types/user-types.js';

export type CallServerEndpointOptions = Partial<{
Expand Down Expand Up @@ -83,7 +82,7 @@ async function callServerEndpoint(
) => Promise<?CallServerEndpoint>,
urlPrefix: string,
sessionID: ?string,
connectionStatus: ConnectionStatus,
isSocketConnected: boolean,
lastCommunicatedPlatformDetails: ?PlatformDetails,
socketAPIHandler: ?SocketAPIHandler,
endpoint: Endpoint,
Expand All @@ -104,7 +103,7 @@ async function callServerEndpoint(

if (
endpointIsSocketPreferred(endpoint) &&
connectionStatus === 'connected' &&
isSocketConnected &&
socketAPIHandler &&
!options?.urlPrefixOverride
) {
Expand Down
9 changes: 4 additions & 5 deletions lib/utils/keyserver-call.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import type { PlatformDetails } from '../types/device-types.js';
import type { Endpoint } from '../types/endpoints.js';
import type { KeyserverInfo } from '../types/keyserver-types.js';
import type { Dispatch } from '../types/redux-types.js';
import type { ConnectionStatus } from '../types/socket-types.js';
import type { CurrentUserInfo } from '../types/user-types.js';

export type CallKeyserverEndpoint = (
Expand Down Expand Up @@ -45,15 +44,15 @@ const baseCreateBoundServerCallsSelector = (
(state: BindServerCallsParams) => state.urlPrefix,
(state: BindServerCallsParams) => state.sessionID,
(state: BindServerCallsParams) => state.currentUserInfo,
(state: BindServerCallsParams) => state.connectionStatus,
(state: BindServerCallsParams) => state.isSocketConnected,
(state: BindServerCallsParams) => state.lastCommunicatedPlatformDetails,
(
dispatch: Dispatch,
cookie: ?string,
urlPrefix: string,
sessionID: ?string,
currentUserInfo: ?CurrentUserInfo,
connectionStatus: ConnectionStatus,
isSocketConnected: boolean,
lastCommunicatedPlatformDetails: ?PlatformDetails,
) =>
bindCookieAndUtilsIntoCallServerEndpoint({
Expand All @@ -62,7 +61,7 @@ const baseCreateBoundServerCallsSelector = (
urlPrefix,
sessionID,
currentUserInfo,
connectionStatus,
isSocketConnected,
lastCommunicatedPlatformDetails,
keyserverID,
}),
Expand Down Expand Up @@ -125,7 +124,7 @@ const bindCallKeyserverEndpointSelector: BindKeyserverCallParams => <
cookie,
urlPrefix,
sessionID,
connectionStatus: connection?.status ?? 'disconnected',
isSocketConnected: connection?.status === 'connected',
lastCommunicatedPlatformDetails,
keyserverID,
});
Expand Down

0 comments on commit cf26c80

Please sign in to comment.