search

aws / amazon-chime-sdk-js

A JavaScript client library for integrating multi-party communications powered by the Amazon Chime service.
Apache License 2.0
704 stars 475 forks source link

Cannot read properties of null (reading 'getVideoTracks') #2659

Open Fnopster opened 1 year ago

Fnopster commented 1 year ago

What happened and what did you expect to happen?

We've received many Cannot read properties of null (reading 'getVideoTracks') errors in Sentry.

It was thrown by this code snippet:

audioVideoDidStart() { 
 this.mediaStreamBroker.mediaStream.getVideoTracks().length>0&&this.contentAudioVideo.videoTileController.startLocalV...

This appears to be from the DefaultContentShareController.

Looking at the code, it appears the ContentShareMediaStreamBroker can set the media stream to null when it cleans up.

Should the DefaultContentShareController check if mediaStream exists before invoking getVideoTracks?

Have you reviewed our existing documentation?

Reproduction steps

No repo steps at this time. Have only seen this error in Sentry.

Amazon Chime SDK for JavaScript version

3.10.0

What browsers are you seeing the problem on?

Chrome

Browser version

Chrome 111.0

Meeting and Attendee ID Information.

No response

Browser console logs

N/A

ltrung commented 1 year ago

That code is triggered when it started not when it cleans up. This seems like a dup of this issue https://github.com/aws/amazon-chime-sdk-js/issues/2535. Do you have any audio sharing use case where there is no video track?

kennyvallejodev commented 1 year ago

@ltrung correct me if i'm wrong please :)

even on a audio sharing (where there's no video track to use) use case, it should have the getAudioTracks available. but in this case is the entire mediaStream from this.mediaStreamBroker that is null.

kennyvallejodev commented 1 year ago

also, the mediaStreamBroker has the mediaStream return type as always defined (get mediaStream(): MediaStream) when actually is initialized as undefined and has a setter that is used when you do the startContentShare.

so why not checking for mediaStream to actually be defined? i actually mentioned this before on my issue #2535

kennyvallejodev commented 1 year ago

@ltrung

I was doing some research in the chime SDK and for this error to happen seems to be a weird race condition where either audioVideoDidStop or stopContentShare is executed first than the audioVideoDidStart because if what you said before on https://github.com/aws/amazon-chime-sdk-js/issues/2535#issuecomment-1397809558 is true about checking on startContentShare that the mediaStream exists before is assigned.

having in mind that we use the DefaultMeetingSession that has all this things by default, we never touch or do any cleanup on the mediaStream.

so even if the fix is easy to fix (safe guard that mediaStream exists on audioVideoDidStart (this is the only place where we are not checking this inside the DefaultContentShareController.ts) so will propose a PR to fix this issue!

kennyvallejodev commented 1 year ago

and also, because this happens inside observers that we don't have control when they're executed, is not possible to actually handle the errors with a tryCatch for saying something. so yeah :(