TS: expect() statements inside event handlers are not taking any effect

[jmillan]

Because mediasoup instances use EnhancedEventEmitter.safeEmit() which absorbs any exception occurred in the event handler.

This makes that expect(1).toBe(2); in a event handler does not fail and makes the test pass.

Example: https://github.com/versatica/mediasoup/blob/v3/node/src/tests/test-DirectTransport.ts#L154

[ibc]

We should only use safeEmit() in internal listeners and never use it in public events. The app is free to add try/catch into its listeners.

NOTE: Same must be done in mediasoup-client and mediasoup-aiortc-client.

NOTE: Let's do this directly in flatbuffers.

[jmillan]

NOTE: Let's do this directly in flatbuffers.

I have just discovered this issue that has always been there. Can we wait for flatbuffers to be moved to v3 and do it then?

It will be impossible for anyone to look at the PR otherwise.

I don't find this being so urgent. We can do it as soon as flatbuffers is merged.

[ibc]

Ok, I was under the impression that you need this done to verify tests in flatbuffers. If not, let's wait.

[ibc]

So I cannot reproduce the error. I mean, I've literally make safeEmit() behave as emit() (no try/catch) and the tests succeed ¯_(ツ)_/¯

@jmillan what do I need to reproduce the problem? you remember?

[jmillan]

So I cannot reproduce the error. I mean, I've literally make safeEmit() behave as emit() (no try/catch) and the tests succeed ¯_(ツ)_/¯

@jmillan what do I need to reproduce the problem? you remember?

As said in the description, write expect(1).toBe(2) in an even handler. The test will pass, and it should not.

[ibc]

Well, that's the thing: if I add this in the test file:

dataConsumer.on('message', (message, ppid) =>
{
  // current code

  expect(1).toBe(2);
});

then I see that those listeners are failing due to the new emittererror event. The thing is that I do NOT see any other error, meaning that all the other expect() are NOT failing!

  console.log
    ------ dataConsumer listener error [eventName:message]: Error: expect(received).toBe(expected) // Object.is equality

    Expected: 2
    Received: 1

      at DataConsumer.<anonymous> (node/src/tests/test-DirectTransport.ts:150:11)

        at DataConsumer.<anonymous> (node/src/tests/test-DirectTransport.ts:235:12)

  console.log
    ------ dataConsumer listener error [eventName:message]: Error: expect(received).toBe(expected) // Object.is equality

    Expected: 2
    Received: 1

      at DataConsumer.<anonymous> (node/src/tests/test-DirectTransport.ts:150:11)

 PASS  node/src/tests/test-DirectTransport.ts
  ✓ router.createDirectTransport() succeeds (11 ms)
  ✓ router.createDirectTransport() with wrong arguments rejects with TypeError (26 ms)
  ✓ directTransport.getStats() succeeds (4 ms)
  ✓ directTransport.connect() succeeds (1 ms)
  ✓ dataProducer.send() succeeds (249 ms)
  ✓ dataProducer.send() with subchannels succeeds (5 ms)
  ✓ DirectTransport methods reject if closed (6 ms)
  ✓ DirectTransport emits "routerclose" if Router is closed (2 ms)
  ✓ DirectTransport emits "routerclose" if Worker is closed (2 ms)

Test Suites: 1 passed, 1 total
Tests:       9 passed, 9 total
Snapshots:   0 total
Time:        3.067 s
Ran all test suites matching /tests\/test-DirectTransport.ts/i.