libp2p / js-libp2p-interfaces

Contains test suites and interfaces you can use to implement the various components of js libp2p.
Other
75 stars 28 forks source link

fix: added promise to PendingDial interface #382

Closed maschad closed 1 year ago

maschad commented 1 year ago

Given that getDialQueue calls the internal connection manager's getDialQueue method, which would return the current pendingDials, it's expected this would include an array of PendingDials containing the promises of connections , this updates the interface to reflect that.

achingbrain commented 1 year ago

Could you add a bit of context here please? What problem does this solve?

maschad commented 1 year ago

Could you add a bit of context here please? What problem does this solve?

My apologies, I have updated the description

achingbrain commented 1 year ago

Not including it in the interface was somewhat intentional to make it harder for callers to modify the internal state of the queue.

This API was meant to allow inspection of the queue only.

On this basis we might be better off excluding it from the list items?

If modification of the queue is desirable one thing that might be worth adding is a cancel method to remove a pending dial from the queue and reject all promises waiting on it?

maschad commented 1 year ago

If modification of the queue is desirable one thing that might be worth adding is a cancel method to remove a pending dial from the queue and reject all promises waiting on it?

I agree with this, although the connection manager is ultimately responsible for handling dials, this would give consumers the option should they not want to use the default connection manager.