Page MenuHomePhabricator
Differential D13325

[lib] update `processOutboundP2PMessages` to return failed message IDs
ClosedPublic
Actions

Authored by kamil on Sep 13 2024, 3:43 AM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Nov 9, 6:14 PM
Unknown Object (File)
Sun, Oct 20, 2:58 PM
Unknown Object (File)
Sun, Oct 20, 2:58 PM
Unknown Object (File)
Sun, Oct 20, 2:57 PM
Unknown Object (File)
Tue, Oct 15, 1:58 AM
Unknown Object (File)
Tue, Oct 15, 1:22 AM
Unknown Object (File)
Mon, Oct 14, 2:29 PM
Unknown Object (File)
Oct 2 2024, 3:32 AM
View All 26 Files
Subscribers
ashoat

Details

Reviewers
tomek
Commits
rCOMMdb45cee21a66: [lib] update `processOutboundP2PMessages` to return failed message IDs
Summary

ENG-8424.

This was confusing, updating to returned failed messages to throw error and handle this as part of *actionTypes.failed.

Depends on D13324

Test Plan

Check that on success of all messages array is empty, otherwise it returns failed messages.

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

kamil created this revision.Sep 13 2024, 3:43 AM
kamil held this revision as a draft.
Herald added a subscriber: ashoat. · View Herald TranscriptSep 13 2024, 3:43 AM
kamil added a child revision: D13326: [lib] introduce type for composable DMOperation.Sep 13 2024, 3:55 AM
Harbormaster completed remote builds in B31593: Diff 44138.Sep 13 2024, 4:00 AM
kamil published this revision for review.Sep 13 2024, 4:35 AM
tomek accepted this revision.Sep 13 2024, 6:19 AM
tomek added inline comments.
lib/tunnelbroker/peer-to-peer-context.js
197–199 ↗(On Diff #44138)

It is quite confusing that this function returns IDs of failed messages. How about modifying the return type to be an object with the IDs, e.g. {+result: 'success'} | {+result: 'failure', +failedMessageIDs: $ReadOnlyArray<string>}?

This revision is now accepted and ready to land.Sep 13 2024, 6:19 AM
kamil mentioned this in D13351: [lib] update `processOutboundP2PMessages` return type.Sep 16 2024, 3:16 AM
kamil added inline comments.Sep 16 2024, 3:20 AM
lib/tunnelbroker/peer-to-peer-context.js
197–199 ↗(On Diff #44138)

That's a good point, but updating this here will cause a lot of changes in the code that will be removed anyway in D13330, to save some time I created D13351 to address this.

Closed by commit rCOMMdb45cee21a66: [lib] update `processOutboundP2PMessages` to return failed message IDs (authored by kamil). · Explain WhySep 16 2024, 4:05 AM
This revision was automatically updated to reflect the committed changes.
kamil added a commit: rCOMMdb45cee21a66: [lib] update `processOutboundP2PMessages` to return failed message IDs.
kamil mentioned this in rCOMM23686a4ec811: [lib] update `processOutboundP2PMessages` return type.

Revision Contents
Changeset List

PathSize
lib/
tunnelbroker/
4 lines

Diff 44220

View Options

lib/tunnelbroker/peer-to-peer-context.js

Loading...