Page MenuHomePhabricator

[keyserver] Start inserting into messages table before calling postMessageSend
ClosedPublic

Authored by ashoat on Feb 18 2023, 12:29 PM.
Tags
None
Referenced Files
F3685341: D6769.id22760.diff
Mon, Jan 6, 10:05 PM
F3685340: D6769.id22702.diff
Mon, Jan 6, 10:05 PM
F3685339: D6769.id22745.diff
Mon, Jan 6, 10:05 PM
F3685327: D6769.id.diff
Mon, Jan 6, 10:04 PM
F3685290: D6769.diff
Mon, Jan 6, 10:00 PM
Unknown Object (File)
Sun, Jan 5, 4:42 PM
Unknown Object (File)
Tue, Dec 24, 3:42 PM
Unknown Object (File)
Tue, Dec 24, 3:42 PM
Subscribers
None

Details

Summary

The await in front of postMessageSend was added in D417. Here's how it works now:

  1. In a script context, we would wait for postMessageSend to complete before inserting into the MySQL messages table
  2. Outside of a script context, we would first call postMessageSend (no await though), and then insert into the MySQL messages table

After this diff, we'll first start inserting into the MySQL messages table (no await), and then call postMessageSend. I think D417 should have been implemented this way from the start, since the insertion into the messages table does not depend on postMessageSend. postMessageSend just needs to be resolved before createMessages resolves.

Depends on D6768

Test Plan

I tested and made sure notifs and associated other things worked correctly

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Harbormaster returned this revision to the author for changes because remote builds failed.Feb 18 2023, 1:34 PM
Harbormaster failed remote builds in B16596: Diff 22702!

Rebase on master to fix CI

Harbormaster returned this revision to the author for changes because remote builds failed.Feb 20 2023, 12:33 PM
Harbormaster failed remote builds in B16634: Diff 22745!
This revision is now accepted and ready to land.Feb 20 2023, 2:33 PM
keyserver/src/creators/message-creator.js
202 ↗(On Diff #22745)

Checked that Promise.all([]) wouldn't fail if undefined (just wasn't sure):

73af3c.png (604×1 px, 137 KB)

and it looks fine.