Skip to content

Commit

Permalink
[lib] Fix keyserver not present in keyserver store error
Browse files Browse the repository at this point in the history
Summary:
issue: https://linear.app/comm/issue/ENG-5905/uncaught-typeerror-cannot-read-properties-of-undefined-reading
The following error was showing up on some peoples consoles:
```
Uncaught (in promise) TypeError: Cannot read properties of undefined (reading 'updatesCurrentAsOf')
    at U7 (prod.4ff39ebc9fcd.build.js:2:4648139)
    at prod.4ff39ebc9fcd.build.js:2:4683479
    at prod.4ff39ebc9fcd.build.js:2:4682129
    at y (prod.4ff39ebc9fcd.build.js:2:1767761)
    at prod.4ff39ebc9fcd.build.js:2:4683981
    at Object.dispatch (prod.4ff39ebc9fcd.build.js:2:2405643)
    at WebSocket.<anonymous> (prod.4ff39ebc9fcd.build.js:2:4669607)
```
The reason was that `setNewSessionActionType` was being dispatched from the Socket without `keyserverID` in payload. The keyserver id is required, but `dispatch` is not typed very well, so flow didn't catch it. This was causing a new keyserver to be added to the keyserver store by the keyserver reducer - a keyserver with id `undefined`. This keyserver was not previously present in the store, so `state.keyserverStore.keyserverInfos[keyserverID].updatesCurrentAsOf` in `baseReducer` would throw.
So the fist thing that has to be done, is fixing the payload.
Secondly, we have an action that can add a keyserver to the store, and this would also result in this error being thrown. But when `addKeyserverActionType` is dispatched we don't need to update `currentAsOf` or `updatesCurrentAsOf`, so we will just if this out (change in master-reducer.js line 87)
Thirdly, we should protect ourselfes from this error in case someone adds some code that can add a keyserver and doesn't update this code (change in master-reducer.js line 109).

Test Plan:
1. Tested that the `setNewSessionActionType` action with broken payload indeed causes this error to show up, by dispatching it manually
2. Tested that adding the `keyserverID` to the payload fixes the issue, and the error does not appear (before changes to `baseReducer`)
3. Tested that the code is not executed when `addKeyserverActionType` is dispatched
4. Tested that even if a new keyserver is added by `setNewSessionActionType` with broken payload, the error is not thrown

Reviewers: ashoat, kamil, tomek

Reviewed By: kamil, tomek

Differential Revision: https://phab.comm.dev/D10091
  • Loading branch information
InkaAlicja committed Nov 29, 2023
1 parent 06cb756 commit 2117945
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 2 deletions.
7 changes: 5 additions & 2 deletions lib/reducers/master-reducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import reduceGlobalThemeInfo from './theme-reducer.js';
import { reduceThreadActivity } from './thread-activity-reducer.js';
import { reduceThreadInfos } from './thread-reducer.js';
import { reduceCurrentUserInfo, reduceUserInfos } from './user-reducer.js';
import { addKeyserverActionType } from '../actions/keyserver-actions.js';
import { siweAuthActionTypes } from '../actions/siwe-actions.js';
import {
registerActionTypes,
Expand Down Expand Up @@ -82,7 +83,8 @@ export default function baseReducer<N: BaseNavInfo, T: BaseAppState<N>>(
action.type !== fullStateSyncActionType &&
action.type !== registerActionTypes.success &&
action.type !== logInActionTypes.success &&
action.type !== siweAuthActionTypes.success
action.type !== siweAuthActionTypes.success &&
action.type !== addKeyserverActionType
) {
for (const keyserverID in keyserverStore.keyserverInfos) {
if (
Expand All @@ -104,8 +106,9 @@ export default function baseReducer<N: BaseNavInfo, T: BaseAppState<N>>(
};
}
if (
state.keyserverStore.keyserverInfos[keyserverID] &&
keyserverStore.keyserverInfos[keyserverID].updatesCurrentAsOf !==
state.keyserverStore.keyserverInfos[keyserverID].updatesCurrentAsOf
state.keyserverStore.keyserverInfos[keyserverID].updatesCurrentAsOf
) {
const keyserverInfos = { ...keyserverStore.keyserverInfos };
keyserverInfos[keyserverID] = {
Expand Down
1 change: 1 addition & 0 deletions lib/socket/socket.react.js
Original file line number Diff line number Diff line change
Expand Up @@ -509,6 +509,7 @@ class Socket extends React.PureComponent<Props, State> {
error: null,
logInActionSource:
logInActionSources.socketAuthErrorResolutionAttempt,
keyserverID: ashoatKeyserverID,
},
});
} else if (!recoverySessionChange) {
Expand Down

0 comments on commit 2117945

Please sign in to comment.