Refresher delete track error

[skmax]

Sometimes Refresher.js cause an error during renegotiation:

TypeError: Cannot read property 'delete' of null
    at EventEmitter.Refresher.ontrackstopped (/usr/src/node_modules/medooze-media-server/lib/Refresher.js:43:16)
    at Object.onceWrapper (events.js:277:13)
    at EventEmitter.emit (events.js:194:15)
    at IncomingStreamTrack.stop (/usr/src/node_modules/medooze-media-server/lib/IncomingStreamTrack.js:486:16)
    at IncomingStream.stop (/usr/src/node_modules/medooze-media-server/lib/IncomingStream.js:417:10)
    at EventEmitter.track.on (/usr/src/components/Participant.js:141:24)
    at EventEmitter.emit (events.js:194:15)
    at IncomingStreamTrack.stop (/usr/src/node_modules/medooze-media-server/lib/IncomingStreamTrack.js:486:16)
    at SDPManagerUnified.processRemoteDescription (/usr/src/node_modules/medooze-media-server/lib/SDPManagerUnified.js:379:13)

It happens sometimes if a stream stops after Refresher.stop() method is triggered

[murillo128]

I am ok with the patch, but this the event stop listener is removed on Refreser.close() so not really sure why it could be happening. Could you investigae it a bit more? I am afraid this could masquerade a different bug.

[skmax]

@murillo128 Unfortunately I wasn't able to find the other reason in the lib quickly, it seems like an async bug. I use a lot of Recorders and recreate them from time to time. The error usually happens when one stream is being deleted and another one adding.

[murillo128]

one question, could it be that you are adding a track twice to the Refresher?

[skmax]

@murillo128 The error occurs if Recorder is stopped from Track stopped event manually.

track.on('stopped', (track) => {
    this.recorder.stop();
});

The stop listener is removed when Refresher is stopped, but as it's being removed when the event is already fired, the handler will be executed anyway. Here's the simplified example:

const EventEmitter = require('events').EventEmitter;

const emitter = new EventEmitter();

const e1 = function () {
  console.log('event1');
  emitter.off('stopped', e2);
};

const e2 = function () {
  console.log('event2');
  emitter.off('stopped', e1);
};

emitter.on('stopped', e1);
emitter.on('stopped', e2);

emitter.emit('stopped'); // event1, event2 logged
emitter.emit('stopped'); // nothing has been logged

In my case, running stopRecorder in the next tick helped.

track.on('stopped', (track) => {
    // setTimeout fix the issue since ontrackstopped is removed in the next tick
    setTimeout(() => this.recorder.stop()); 
});

@murillo128 let me know what you think. I guess this null check might be useful in this case

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}