versatica / mediasoup

Cutting Edge WebRTC Video Conferencing
https://mediasoup.org
ISC License
6.18k stars 1.12k forks source link

Issues in Node tests #1306

Closed ibc closed 8 months ago

ibc commented 8 months ago

Here some issues in Node tests:

Probably many similar ones if we run each test file separately.

NOTE: Yes, I added --detectOpenHandles to jest command, but it should work fine.

ibc commented 8 months ago

NOTE: In tests above I'm doing this change to avoid more open handlers when a test file completes:

diff --git a/node/src/Channel.ts b/node/src/Channel.ts
index 7e1c71961..abb2f0468 100644
--- a/node/src/Channel.ts
+++ b/node/src/Channel.ts
@@ -227,16 +227,6 @@ export class Channel extends EnhancedEventEmitter {
        this.#producerSocket.removeAllListeners('end');
        this.#producerSocket.removeAllListeners('error');
        this.#producerSocket.on('error', () => {});
-
-       // Destroy the socket after a while to allow pending incoming messages.
-       setTimeout(() => {
-           try {
-               this.#producerSocket.destroy();
-           } catch (error) {}
-           try {
-               this.#consumerSocket.destroy();
-           } catch (error) {}
-       }, 200);
    }
nazar-pc commented 8 months ago

NOTE: In tests above I'm doing this change to avoid more open handlers when a test file completes

I strongly dislike timeouts like that. They are not reliable and will definitely fail at some point.

ibc commented 8 months ago

NOTE: In tests above I'm doing this change to avoid more open handlers when a test file completes

I strongly dislike timeouts like that. They are not reliable and will definitely fail at some point.

Yes. It was the only one in the code.