daily-co / daily-react

React hooks and components to get started with Daily
https://docs.daily.co/reference/daily-react
BSD 2-Clause "Simplified" License
52 stars 16 forks source link

BUG - useLocalSessionId does not update after leaving the room then calling daily.preAuth #9

Closed ssimonitch closed 1 year ago

ssimonitch commented 1 year ago

Expected behavior

After calling daily.preAuth I see that the useLocalSessionId hook correctly returns the session ID of the local user. I can then call daily.join() to the join the room with that session ID, and then call daily.leave() to leave the room with that session ID.

At this point the useLocalSessionId hook returns undefined, and I expect that calling preAuth again will issue a new local session ID that can be accessed by the useLocalSessionId hook.

Describe the bug (unexpected behavior)

In the above scenario, if I call preAuth again after leaving the room I see a participant-updated event fire with a new session ID for the local user but the return value of useLocalSessionId as well as useLocalParticipant remains undefined. I would expect those values to reflect the new local session ID, unless I am misunderstanding how this works?

Steps to reproduce

  1. Call daily.preAuth() passing in url and token as options
  2. Notice participant-updated event fires with new session ID, which is correctly returned by useLocalSessionId and useLocalParticipant hooks
  3. Call daily.join() to join room
  4. Call daily.leave() to leave room
  5. Call daily.preAuth again, passing in same url and token options
  6. Notice participant-updated event fires with new session ID, however useLocalSessionId and useLocalParticipant hooks returns undefined

Additional context

Room I am testing in: https://tastemade-staging.daily.co/zPmtkqdArvkU4a53DdzC

Regaddi commented 1 year ago

Thank you, @ssimonitch, for the bug report!

I verified the issue and am working on a fix.

Regaddi commented 1 year ago

Update

We're planning to push out a release with this fix today or Monday.

Regaddi commented 1 year ago

This bug has been fixed in daily-react 0.7.0 today!

ssimonitch commented 1 year ago

Awesome, thank you!

ssimonitch commented 1 year ago

@Regaddi I'm seeing a different issue now:

  1. Call daily.preAuth() passing in url and token as options
  2. Notice useLocalSessionId returns session id value
  3. Call daily.join() to join room
  4. Call daily.leave() to leave room
  5. Notice useLocalSessionId returns undefined and then updates again to return the previous session id value despite no longer being in the room

This also occurs if I don't call daily.join but just daily.leave() after pre-auth, or even if I destroy the call object. Is this intended or a new bug?

Regaddi commented 1 year ago

Hey @ssimonitch,

thanks for reporting back! This doesn't sound intended, indeed. I tried to reproduce the issue, but it seems I'm missing some context or something is different in your setup.

Here's the Codesandbox I've been using for testing: https://codesandbox.io/s/dazzling-phoebe-4bvkjo

Could you try using that sandbox or provide a fork where the issue can be reproduced?

Thank you in advance!

ssimonitch commented 1 year ago

Hey @Regaddi

Thanks for taking the time to look into this and share the Code Sandbox. I've forked it and was able to reproduce the issue by updated the call object initialization and join/leave behavior to be similar to what I'm working with in my current project.

https://codesandbox.io/s/inspiring-brown-upky9o

It appears that the call to callObject.updateParticipant("local", { setAudio: false, setVideo: false }); right before leaving the room is the root cause. I've confirmed that removing this from the code I'm working on now also fixes the issue. Perhaps updating the participant immediately before they leave the room triggers some sort of race condition?

I had originally added that call because I was also using useDevices to render a device picker and noticed that after leaving the room the device state returned by that hook did not update, which I realized was because once I left the room and destroyed the call object I would no longer be getting state updates from that hook. So I added in that updateParticipant call to try and revert the camState and micState back to the new idle state which would reset my device picker UI but even that wasn't perfect because while camState reverted to idle I saw that micState remains granted and I see that the microphone device's selected value also remains as true. I'm not sure if this particular behavior is expected or a separate bug.

In any case, I solved the above issue by simply hiding the device picker component unless we have a local session ID, so I suppose I no longer need to make that call to updateParticipant before leaving the room, but it would be good to understand why this is happening.

In the meantime I've just been using my own hook to track the local participant ID which has been behaving as I expected, but I'd of course rather use the library hook:

export const useLocalParticipantId = (): string | undefined => {
  const [localParticipantId, setLocalParticipantId] = useState<string>();

  useDailyEvent(
    'participant-updated',
    useCallback(
      ({ participant }: DailyEventObjectParticipant) => {
        if (participant.local && participant.session_id !== localParticipantId) {
          setLocalParticipantId(participant.session_id);
        }
      },
      [localParticipantId],
    ),
  );

  useDailyEvent(
    'left-meeting',
    useCallback(() => setLocalParticipantId(undefined), []),
  );

  return localParticipantId;
};
Regaddi commented 1 year ago

@ssimonitch Thank you very much for the detailed description of the issue!

In fact this seems like a bug in Daily React: Currently we handle the left-meeting event separately from participant-* events. In the case you described, the participant-updated events emitted for the call to daily.updateParticipant might be handled after the left-meeting event, due to the event throttling.

I'll add a fix to handle the left-meeting event as part of the same throttling queue, so that all events in DailyParticipants are handled in order.

Regaddi commented 1 year ago

@ssimonitch The above mentioned issue has been fixed with 0.7.1

ssimonitch commented 1 year ago

Fantastic, thank you!