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
F2755276: D13049.diff
Wed, Sep 18, 9:36 PM
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
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 ↗(On Diff #43379)

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 ↗(On Diff #43379)

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 ↗(On Diff #43379)

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 ↗(On Diff #43379)

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 ↗(On Diff #43379)

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 ↗(On Diff #43379)

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 ↗(On Diff #43379)

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.