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
F2715463: D13049.id43309.diff
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
Lint
Lint Not Applicable
Unit
Tests Not Applicable

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.Mon, Aug 19, 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.