Page MenuHomePhabricator

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

Authored by ashoat on Jan 18 2024, 12:51 AM.
Tags
None
Referenced Files
F3380282: D10696.diff
Wed, Nov 27, 10:39 PM
Unknown Object (File)
Sun, Nov 17, 1:41 PM
Unknown Object (File)
Wed, Nov 13, 6:15 AM
Unknown Object (File)
Wed, Nov 13, 6:15 AM
Unknown Object (File)
Wed, Nov 13, 6:15 AM
Unknown Object (File)
Wed, Nov 13, 6:14 AM
Unknown Object (File)
Wed, Nov 13, 6:13 AM
Unknown Object (File)
Oct 22 2024, 4:57 PM
Subscribers
None

Details

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:

  1. 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
  2. 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

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
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