Page MenuHomePhabricator
Differential D13325

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

Authored by kamil on Fri, Sep 13, 3:43 AM.
Tags
None
Referenced Files
F2748441: D13325.id44138.diff
Wed, Sep 18, 10:23 AM
Unknown Object (File)
Mon, Sep 16, 5:50 PM
Unknown Object (File)
Sat, Sep 14, 5:54 AM
Unknown Object (File)
Sat, Sep 14, 12:31 AM
Unknown Object (File)
Fri, Sep 13, 9:13 PM
Unknown Object (File)
Fri, Sep 13, 4:44 PM
Unknown Object (File)
Fri, Sep 13, 8:44 AM
Unknown Object (File)
Fri, Sep 13, 8:43 AM
View All 9 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.Fri, Sep 13, 3:43 AM
kamil held this revision as a draft.
Herald added a subscriber: ashoat. · View Herald TranscriptFri, Sep 13, 3:43 AM
kamil added a child revision: D13326: [lib] introduce type for composable DMOperation.Fri, Sep 13, 3:55 AM
Harbormaster completed remote builds in B31593: Diff 44138.Fri, Sep 13, 4:00 AM
kamil published this revision for review.Fri, Sep 13, 4:35 AM
tomek accepted this revision.Fri, Sep 13, 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.Fri, Sep 13, 6:19 AM
kamil mentioned this in D13351: [lib] update `processOutboundP2PMessages` return type.Mon, Sep 16, 3:16 AM
kamil added inline comments.Mon, Sep 16, 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 WhyMon, Sep 16, 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...