Page MenuHomePhabricator

[lib] process Inbound P2P messages on app start
ClosedPublic

Authored by kamil on Aug 12 2024, 6:49 AM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Sep 16, 3:18 AM
Unknown Object (File)
Sun, Sep 15, 9:07 AM
Unknown Object (File)
Sun, Sep 15, 9:06 AM
Unknown Object (File)
Fri, Sep 13, 2:15 PM
Unknown Object (File)
Tue, Sep 10, 6:51 PM
Unknown Object (File)
Tue, Sep 10, 8:06 AM
Unknown Object (File)
Mon, Sep 9, 11:54 PM
Unknown Object (File)
Thu, Sep 5, 11:24 AM
Subscribers

Details

Summary

ENG-8902.

Depends on D13047

Test Plan
  1. Kill app after decrypting but before processing (adding sleep)
  2. Open and make sure message is processed

Diff Detail

Repository
rCOMM Comm
Branch
resending-4
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

kamil held this revision as a draft.
kamil published this revision for review.Aug 12 2024, 6:54 AM
kamil edited reviewers, added: bartek; removed: marcin.
This revision is now accepted and ready to land.Aug 19 2024, 2:17 AM
tomek added inline comments.
lib/tunnelbroker/peer-to-peer-message-handler.js
42–44

I'm wondering if we could encode these two booleans into some state variables, e.g. BEFORE_INBOUND_MESSAGES_PROCESSING | INBOUND_MESSAGES_PROCESSING | WAITING_FOR_A_MESSAGE | PROCESSING_A_MESSAGE

98

Why do we have to process them on app start? Do we assume that some messages could be in the middle of processing when the app got closed?

lib/tunnelbroker/use-peer-to-peer-message-handler.js
119–148

Why do we have to remove the messages? Why are we doing this only in these two places?

lib/tunnelbroker/peer-to-peer-message-handler.js
42–44

I think this approach is a bit more readable because processing inbound messages on app start and processing received messages are two different things

98

This is to handle cases when the app was killed after decrypting but before processing - we can decrypt the message only once, so in one transaction we decrypt and save the Inbound message. That saves us from losing data.

lib/tunnelbroker/use-peer-to-peer-message-handler.js
119–148

LOG_OUT_PRIMARY_DEVICE - we're about to delete the database anyway
DM_OPERATION - the message is removed in DBOpsHandler after processing it on the store
and remaining two has to be handled here

lib/tunnelbroker/use-peer-to-peer-message-handler.js
119–148

I wonder if this could be documented in the code via code comments

This revision was landed with ongoing or failed builds.Wed, Aug 21, 2:57 AM
This revision was automatically updated to reflect the committed changes.