twilio / twilio-video-app-react

A collaboration application built with the twilio-video.js SDK and React.js
Apache License 2.0
1.8k stars 725 forks source link

Local participant disconnected error after using EndCallButton to disconnect #742

Open cindyloo opened 2 years ago

cindyloo commented 2 years ago

code from Twilio React sample app

export default function EndCallButton(props: { className?: string }) { // room coming through props
  const classes = useStyles();
  const vRoom = useRoomState();
  const handleClick = (e, vRoom, room) => { // I added this in hopes that I could access the room at the time of disconnection
    if (!isEmpty(vRoom)) {
      if (room.localParticipant && vRoom === "connected") {
        try {
          room && room.localParticipant.state === 'connected' && room.disconnect()
        } catch (e) {
          console.log("endcall button caught error");
        }
      } else {
        console.log("Error participant already disconnected");
      }
    }
  };

  //useEffect(() => {
  //}, vRoom);
  return (
    <Button
      onClick={e => handleClick(e, vRoom, props.room)}
      className={clsx(classes.button, props.className)}
      data-cy-disconnect
    >
      Disconnect
    </Button>
  );
}

Expected behavior:

I'd expect to disconnect and receive no error

TODO

    at eval (room.js?5fe1:244:1)
    at Map.forEach (<anonymous>)
    at RoomV2._disconnect (room.js?5fe1:243:1)
    at RoomSignaling.disconnect (room.js?b982:204:1)
    at Room.disconnect (room.js?47f1:174:1)
    at handleClick (EndCallButton.tsx?bb64:29:71)
    at onClick (EndCallButton.tsx?bb64:43:21)
    at HTMLUnknownElement.callCallback (react-dom.development.js?ac89:188:1)
    at Object.invokeGuardedCallbackDev (react-dom.development.js?ac89:237:1)
    at invokeGuardedCallback (react-dom.development.js?ac89:292:1)
    at invokeGuardedCallbackAndCatchFirstError (react-dom.development.js?ac89:306:1)
    at executeDispatch (react-dom.development.js?ac89:389:1)
    at executeDispatchesInOrder (react-dom.development.js?ac89:414:1)
    at executeDispatchesAndRelease (react-dom.development.js?ac89:3278:1)
    at executeDispatchesAndReleaseTopLevel (react-dom.development.js?ac89:3287:1)
    at forEachAccumulated (react-dom.development.js?ac89:3259:1)
    at runEventsInBatch (react-dom.development.js?ac89:3304:1)
    at runExtractedPluginEventsInBatch (react-dom.development.js?ac89:3514:1)
    at handleTopLevel (react-dom.development.js?ac89:3558:1)
    at batchedEventUpdates$1 (react-dom.development.js?ac89:21871:1)
    at batchedEventUpdates (react-dom.development.js?ac89:795:1)
    at dispatchEventForLegacyPluginEventSystem (react-dom.development.js?ac89:3568:1)
    at attemptToDispatchEvent (react-dom.development.js?ac89:4267:1)
    at dispatchEvent (react-dom.development.js?ac89:4189:1)
    at unstable_runWithPriority (scheduler.development.js?c964:653:1)
    at runWithPriority$1 (react-dom.development.js?ac89:11039:1)
    at discreteUpdates$1 (react-dom.development.js?ac89:21887:1)
    at discreteUpdates (react-dom.development.js?ac89:806:1)
    at dispatchDiscreteEvent (react-dom.development.js?ac89:4168:1)

Software versions:

cindyloo commented 2 years ago
Screen Shot 2022-08-15 at 6 35 51 AM
cindyloo commented 2 years ago

debug logs from Twilio if helpful:

2022-08-16T19:17:13.257Z debug [RemoteAudioTrack twilio/twilio-video.js#18: MT278d00ef872f90ab57e2ab75305d1a54] Ended
log.js?a324:167 2022-08-16T19:17:13.257Z debug [RemoteVideoTrack twilio/twilio-video.js#19: MTc7382970360af265e1538fc0712b3625] Ended
log.js?a324:167 2022-08-16T19:17:13.258Z debug [RemoteVideoTrack twilio/twilio-video.js#20: MT7d37ab7b378a74548b748c1641b52d1c] Ended
log.js?a324:167 2022-08-16T19:17:13.258Z info [MediaSignaling twilio/twilio-video.js#1:network_quality] Tearing down
log.js?a324:167 2022-08-16T19:17:13.259Z info [MediaSignaling twilio/twilio-video.js#3:publisher_hints] Tearing down
log.js?a324:167 2022-08-16T19:17:13.259Z info [MediaSignaling #0:active_speaker] Tearing down
log.js?a324:167 2022-08-16T19:17:13.259Z info [MediaSignaling twilio/twilio-video.js#5:track_switch_off] Tearing down
log.js?a324:167 2022-08-16T19:17:13.260Z info [MediaSignaling twilio/twilio-video.js#2:render_hints] Tearing down
log.js?a324:167 2022-08-16T19:17:13.260Z info [MediaSignaling twilio/twilio-video.js#4:track_priority] Tearing down
2room.js:255 Uncaught (in promise) Error: LocalParticipant disconnected
    at eval (webpack-internal:///./node_modules/twilio-video/es5/signaling/v2/room.js:255:42)
    at Map.forEach (<anonymous>)
    at RoomV2._disconnect (webpack-internal:///./node_modules/twilio-video/es5/signaling/v2/room.js:254:38)
    at RoomSignaling.disconnect (webpack-internal:///./node_modules/twilio-video/es5/signaling/room.js:204:21)
    at Room.disconnect (webpack-internal:///./node_modules/twilio-video/es5/room.js:178:25)
    at disconnect (webpack-internal:///./src/components/VideoProvider/useRoom/useRoom.tsx:71:32)
eval @ room.js:255
RoomV2._disconnect @ room.js:254
RoomSignaling.disconnect @ room.js:204
Room.disconnect @ room.js:178
disconnect @ useRoom.tsx:71
index.tsx?72be:115 VideoProvider {type: 'none'}
PikaJoyce commented 2 years ago

Hi @cindyloo,

Thank you for opening this issue with us. It seems like this is an issue with the twilio-video react app. I'll be transferring this over to our wonderful folks there!

Best, Joyce

timmydoza commented 2 years ago

Hi @cindyloo - thanks for the issue!

I've been looking into this issue. The error that you are seeing is not very descriptive, but it is thrown only when you try to disconnect from a room while a local track is being published to the room. To demonstrate this, I found that the following code snippet can reproduce the issue:

twilioRoom.localParticipant.publishTrack(new TwilioVideo.LocalDataTrack()); twilioRoom.disconnect();

In this snippet, the publishTrack() function is an asynchronous operation, so it doesn't finish executing before room.disconnect() is called. This produces the same "LocalParticipant disconnected" error that you are seeing.

This makes me wonder - is a track in the process of being published when room.disconnect is called?

Some questions that I have - have you modified useRoom.tsx? I see in the stack trace that it references line 71 of that file, but in the master branch of this repo, there isn't really anything there. If you've changed this file, could you please share the changes so that we could take a look?

Also - in the debug logs - can you share some more of the logos that you see before the disconnect happens? The debug logs should show you which track is in the process of being published when the disconnect happens. For instance, when I run the snippet above to reproduce the error - I see this in the logs:

react_devtools_backend.js:4026 2022-08-22T21:38:56.795Z warn [LocalParticipant #2: PA3ffbea7f7684089a6f4607a1022a5d16] Failed to publish the LocalDataTrack: LocalParticipant disconnected

Do you see anything like this in the logs?

cindyloo commented 2 years ago

hi @timmydoza! The changes I made to useRoom are minor afaict:

insights: true, when connecting via to Video.connect

a wrapper around the Video assignment if (typeof window !== "undefined") { window.TwilioVideo = Video; } as I'm using next.js

and an extension to maxListeners: newRoom.setMaxListeners(20);

at EndRoomDisconnect, the only thing I'm trying to do is react to the remaining (local) track and to continue to draw a video mask over it (we anonymize our videos and create a canvas track to do so). If the Room has reloaded after a Remote participant disconnected- and there is only the LocalParticipant present, and that my VideoTrack component has re-attached to the localParticipant video element, it is possible that it's video mask canvas track has been told to re-publish as well... although that is the right behavior, if the re-publishing doesn't happen fairly quickly for some reason, a race condition could be causing this error?

here is the code in particular ( in what I am calling AnonymousVideoTrack

useEffect(() => {
    const currentRef = forwardRef?.current; // to keep a hold of a ref created at app level
    const canvasOverlayBuff = canvasRef?.current;
    if (currentRef && canvasOverlayBuff) {
      currentRef.muted = true;
      currentRef.playsInline = true; // for ios bug
      currentRef.autoPlay = true;

      if (track.setPriority && priority) {
        track.setPriority(priority);
      }

      if (utils.supportsOffscreenCanvasFilters) {
        setProcessor(grayScaleProcessor, track);
      }
      track.attach(currentRef);

      console.log(
        " ANON track attached to el " +
          currentRef.id +
          " trac " +
          JSON.stringify(track)
      );

      // avoid trying to add track twice. have to have a video track and canvas overlay
      // before we can attach overlay
      if (
        canvasOverlayBuff &&
        allPublishedTracksForParticipant // ensure particpant has published tracks before we add canvas track
      ) {
        const anonCanvasTrack = getLocalVideoTrackFromCanvas( // same pattern as in useLocalTracks
          participant.sid,
          canvasOverlayBuff
        ).then((track: LocalVideoTrack) => {
          if (supportsCPublishing) { // this is special cased b/c WIndows 10 throws a client media description error
            (participant as LocalParticipant)
              .publishTrack(track)
              .then(localTrackPublication => {
                console.log(
                  `Track overlay was published with SID ${localTrackPublication.trackSid} for ${participant.sid}`
                );
              });
          }
        });
    return () => {
      track.detach(currentRef);  // maybe I comment this out if this is a possible issue? 
      console.log("detaching anon track");  

      if (currentRef) {
        currentRef.srcObject = null;
      }

      if (track.setPriority && priority) {
        // Passing `null` to setPriority will set the track's priority to that which it was published with.
        track.setPriority(null);
      }
    };
  }, [track, priority, participant, forwardRef, canvasRef]);

I will look in the logs and get back to you also

thank you for your help!

cindyloo commented 2 years ago

hi again- I have made sure to clean up my tracks, both videoTracks and tracks published via the canvas stream. I do still get the "local participant disconnected error" and I see that the app is re-rendering, which would try to make it re-publish. Not sure how it is getting past the participant==null test though

cindyloo commented 1 year ago

Our local fix of the twilio-react-app was the following:

The first part of the fix was to separate track attach/detach logic from publish/unpublish logic in (Anonymous)VideoTrack, and handle the publish promise more gracefully. This fixes the "Unhandled Promise" part of the bug.

That got the "LocalParticipant disconnected" error to show up in the ErrorDialog component. The error itself is coming from useHandleTrackPublicationFailed In the VideoProvider, where I added a second error handler that just logs the error into the console for now. At least users won't see it, but I'm not sure where the underlying bug comes from yet (in Twilio).

Also added a doDisconnect function to share the disconnect code between useHandleRoomDisconnection and DeviceSelectionScreen.