Not using pair key in pipeToRouter()

[piranna]

At

mediasoup/node/src/Router.ts

Line 725 in ec186c5

const pipeTransportPairKey = router.id;

pipeToRouter() is using only destination Router ID as key of the pair of Router instances, as it was discussed in #697. Later on

mediasoup/node/src/Router.ts

Lines 810 to 813 in ec186c5

	this.#mapRouterPairPipeTransportPairPromise.set(
	pipeTransportPairKey, pipeTransportPairPromise);
	router.addPipeTransportPair(this.id, pipeTransportPairPromise);

it's being stored at mapRouterPairPipeTransportPairPromise with router.id and at addPipeTransportPair() with this.id, so not using pair key at all but just storing them twice (equally acceptable) and removing it later on both

mediasoup/node/src/Router.ts

Lines 774 to 786 in ec186c5

	localPipeTransport.observer.on('close', () =>
	{
	remotePipeTransport.close();
	this.#mapRouterPairPipeTransportPairPromise.delete(
	pipeTransportPairKey);
	});
	remotePipeTransport.observer.on('close', () =>
	{
	localPipeTransport.close();
	this.#mapRouterPairPipeTransportPairPromise.delete(
	pipeTransportPairKey);
	});

and

mediasoup/node/src/Router.ts

Lines 949 to 966 in ec186c5

	pipeTransportPairPromise
	.then((pipeTransportPair) =>
	{
	const localPipeTransport = pipeTransportPair[this.id];
	// NOTE: No need to do any other cleanup here since that is done by the
	// Router calling this method on us.
	localPipeTransport.observer.on('close', () =>
	{
	this.#mapRouterPairPipeTransportPairPromise.delete(
	pipeTransportPairKey);
	});
	})
	.catch(() =>
	{
	this.#mapRouterPairPipeTransportPairPromise.delete(
	pipeTransportPairKey);
	});

. Is this right? The rest of the pair selection code seems to be ok, just only missing the [this.id, router.id].sort().join(',') part, isn't it? Or am I missing something? Are we storing the pair in local pairs and also in remote Router ones instead of having a common list with unique IDs? Is it just not using a global list the only reason of doing it that way?

[ibc]

Please let's not open issues in GH for asking questions or having discussions.

Are we storing the pair in local pairs and also in remote Router ones instead of having a common list with unique IDs? Is it just not using a global list the only reason of doing it that way?

Yes

[piranna]

Please let's not open issues in GH for asking questions or having discussions.

Yeah, sorry for that, I first was writting a bug report since only saw the first const pipeTransportPairKey = router.id line and got confused with it being the key and though it was a bug, but later while writting the bug issue I was collecting references on the code to see if there was something I was overlooking, that's why finally I asked about it to ensure my conclusions were right, code got to be a bit complex to understand in this feature...

[ibc]

Code got a bit complex to avoid having global state, but not that much.

[piranna]

Code got a bit complex to avoid having global state, but not that much.

Logic is fine, and I really like the aproach, very smart :-) It's more about code structure, that Promise with Promise.then() inside... ugh :-/