[lib] Don't delete pending messages during session recovery
ClosedPublic
Actions

Authored by ashoat on Jan 18 2024, 12:51 AM.

Details

Reviewers

tomek
inka
atul

Commits

rCOMM077530051e77: [lib] Don't delete pending messages during session recovery

Summary

Noticed that the pending messages in my test plan would get erased on LOG_IN_SUCCESS during session recovery. This diff makes sure we don't lose those pending messages.

NOTE: I wanted to check if the logged-in user has changed, but we can't make it until ENG-6126 is complete. That said, this should be a redundant check.

Depends on D10695

Test Plan

I tested this stack using the following procedure:

I tested primarily on native
1. I compiled a dev build and deployed it to an iOS simulator
2. I created a brand new account on my local keyserver using the iOS app
3. I ran Redux dev tools: cd native && yarn redux-devtools
4. I added a 30s sleep at the start of resolveKeyserverSessionInvalidation
5. I made KeyserverConnectionsHandler return null so that the socket wouldn’t automatically recover the session prior to my testing
6. I killed the app
7. I deleted all of the test user’s cookie
8. I then opened the app again and navigated to a chat and sent two messages
9. By following the Redux monitor, I was able to see that the keyserver session invalidation recovery was successful, and both messages were eventually sent after the 30s sleep concluded
On web, we don’t support keyserver session invalidation. However, I tested to make sure that the web app still loaded after my changes

Diff Detail

Repository

rCOMM Comm

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

ashoat created this revision.Jan 18 2024, 12:51 AM

ashoat added a child revision: D10697: [lib] Fix order of resolving keyserver calls after session recovery.

Harbormaster completed remote builds in B25872: Diff 35767.Jan 18 2024, 1:32 AM

ashoat requested review of this revision.Jan 18 2024, 1:32 AM

tomek accepted this revision.Jan 18 2024, 6:57 AM

tomek added inline comments.

lib/reducers/message-reducer.js
747	If we want to keep this log, we should make it more descriptive. But probably we don't need it.

This revision is now accepted and ready to land.Jan 18 2024, 6:57 AM

ashoat added inline comments.Jan 18 2024, 8:23 AM

lib/reducers/message-reducer.js
747	Oops, that was a mistake left over from testing! Thanks for pointing it out

Remove console.log and use a couple shorthands

Harbormaster completed remote builds in B25908: Diff 35809.Jan 18 2024, 10:11 AM

Closed by commit rCOMM077530051e77: [lib] Don't delete pending messages during session recovery (authored by ashoat). · Explain WhyJan 18 2024, 10:54 AM

This revision was automatically updated to reflect the committed changes.

ashoat added a commit: rCOMM077530051e77: [lib] Don't delete pending messages during session recovery.

Revision Contents
Changeset List

Path

Size

lib/

reducers/

message-reducer.js

22 lines

Diff 35767

View Options

lib/reducers/message-reducer.js

Show First 20 Lines • Show All 573 Lines • ▼ Show 20 Lines	async keyserverAuthInfo => {

const responses: { +[string]: ClientLogInResponse } =		const responses: { +[string]: ClientLogInResponse } =
await callKeyserverEndpoint(		await callKeyserverEndpoint(
'keyserver_auth',		'keyserver_auth',
requests,		requests,
keyserverAuthCallSingleKeyserverEndpointOptions,		keyserverAuthCallSingleKeyserverEndpointOptions,
);		);

const userInfosArrays = [];

let threadInfos: RawThreadInfos = {};		let threadInfos: RawThreadInfos = {};
const calendarResult: WritableCalendarResult = {		const calendarResult: WritableCalendarResult = {
calendarQuery: keyserverAuthInfo.calendarQuery,		calendarQuery: keyserverAuthInfo.calendarQuery,
rawEntryInfos: [],		rawEntryInfos: [],
};		};
const messagesResult: WritableGenericMessagesResult = {		const messagesResult: WritableGenericMessagesResult = {
messageInfos: [],		messageInfos: [],
truncationStatus: {},		truncationStatus: {},
Show All 21 Lines	for (const keyserverID in responses) {
messagesResult.currentAsOf = {		messagesResult.currentAsOf = {
...messagesResult.currentAsOf,		...messagesResult.currentAsOf,
[keyserverID]: responses[keyserverID].serverTime,		[keyserverID]: responses[keyserverID].serverTime,
};		};
updatesCurrentAsOf = {		updatesCurrentAsOf = {
...updatesCurrentAsOf,		...updatesCurrentAsOf,
[keyserverID]: responses[keyserverID].serverTime,		[keyserverID]: responses[keyserverID].serverTime,
};		};
userInfosArrays.push(responses[keyserverID].userInfos);
userInfosArrays.push(responses[keyserverID].cookieChange.userInfos);
}		}

		const userInfosArrays = [
		responses[authoritativeKeyserverID()].userInfos,
		responses[authoritativeKeyserverID()].cookieChange.userInfos,
		tomekUnsubmitted Not Done Inline Actions I don't think it is guaranteed that there will always be a response from the authoritative keyserver, e.g. when adding a new keyserver. We probably should add a condition here and extend a test plan to verify a non-auth keyserver auth. tomek: I don't think it is guaranteed that there will always be a response from the authoritative…
		];

const userInfos = mergeUserInfos(...userInfosArrays);		const userInfos = mergeUserInfos(...userInfosArrays);

return {		return {
threadInfos,		threadInfos,
currentUserInfo: responses[authoritativeKeyserverID()]?.currentUserInfo,		currentUserInfo: responses[authoritativeKeyserverID()]?.currentUserInfo,
calendarResult,		calendarResult,
messagesResult,		messagesResult,
userInfos,		userInfos,
▲ Show 20 Lines • Show All 526 Lines • Show Last 20 Lines

[lib] Don't delete pending messages during session recoveryClosedPublicActions