Page MenuHomePhabricator

[lib] Make sure PeerToPeerMessageHandler is run after persisted state is read
ClosedPublic

Authored by angelika on Oct 28 2024, 2:48 PM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Dec 19, 1:50 AM
Unknown Object (File)
Sun, Dec 15, 7:56 PM
Unknown Object (File)
Sun, Dec 15, 7:56 PM
Unknown Object (File)
Sun, Dec 15, 7:56 PM
Unknown Object (File)
Sun, Dec 15, 7:56 PM
Unknown Object (File)
Sun, Dec 15, 7:56 PM
Unknown Object (File)
Sun, Dec 15, 7:54 PM
Unknown Object (File)
Sun, Dec 15, 7:50 PM
Subscribers

Details

Summary
Test Plan
  1. Use logs to verify if PeerToPeerMessageHandler waits for persisted state
  2. Comment out sqliteAPI.removeInboundP2PMessages and send some messages and restart the app
  3. Errors like invalid operation should appear
  4. After this patch there should be less of them

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

Question about the test plan:

After this patch there should be less of them

Did some of errors still appear? What's your theory for why that is?

Question about the test plan:

After this patch there should be less of them

Did some of errors still appear? What's your theory for why that is?

Yes, they still appeared probably because this one: https://phab.comm.dev/D13792 has not landed yet. When I was testing that one the errors also disappeared.

Makes sense, thanks for explaining!

This revision is now accepted and ready to land.Oct 29 2024, 5:22 AM

Rebase and implement requested changes

You can't update the commit message with arc diff. Instead, you should edit the revision on Phabricator, and then run arc amend locally to update the commit text based on what's on Phabricator. (I think this last step also gets run automatically as part of arc land.)

You can't update the commit message with arc diff. Instead, you should edit the revision on Phabricator, and then run arc amend locally to update the commit text based on what's on Phabricator. (I think this last step also gets run automatically as part of arc land.)

oh, ok, I wanted to do just arc amend, I thought I need to do this manually 🤦‍♀️