search

daily-co / daily-js

https://docs.daily.co/reference/daily-js
BSD 2-Clause "Simplified" License
103 stars 33 forks source link

error handling sdp offer: TypeError: Attempted to assign to readonly property. - "SESSION_ID" #227

Closed troutowicz closed 1 year ago

troutowicz commented 1 year ago

Expected behavior

Joining a call should not crash on Safari.

Describe the bug (unexpected behavior)

  1. User A (Safari) joins a call
  2. User B (any browser) joins the call
  3. User A subscribes to User B audio/video
  4. The meeting ends in error for User A

System information

* Device: MacBook Air M2 * OS, version: Sonoma * Browser, version: Safari 17.0 (19616.1.27.111.16) # Additional context ![CleanShot 2023-09-20 at 12 59 03](https://github.com/daily-co/daily-js/assets/8117994/5bfc8cd8-8dc2-4ba5-a769-2d19e9238e5d) ![CleanShot 2023-09-20 at 12 59 41](https://github.com/daily-co/daily-js/assets/8117994/756f8085-4233-46c9-91fb-8f45ee22086e) I have seen a couple forms of the error. Both stack traces have `setRemoteDescription` call in common. Using `daily-js 0.49.1`.
troutowicz commented 1 year ago

We use manual subscriptions. Referencing the demo repo I noticed forcing network topology to sfu. Trying this fixed the issue for us.

The docs recommend not doing this, however. For an implementation that generally always has 3 or more participants (at which daily would automatically change to sfu), is it acceptable to force sfu from the start?

Edit:

I see here mentions SFU is required for manual track subscriptions. This hasn't been the case on chromium based browsers. Is it a Safari specific issue?

Edit 2:

Forcing SFU degrades joining a call significantly on chromium based browsers. Join time grew by 5-6 seconds. Is there a problem here that Daily can resolve for Safari P2P?

troutowicz commented 1 year ago

Any update on this? Thank you.

mattieruth commented 1 year ago

Thank you for the report and apologies this slipped through the cracks. Looking into it now.

mattieruth commented 1 year ago

Following your steps on the same mac/safari setup, I'm unable to reproduce your issue. Can you reproduce it again for me but after you load your page and before you join the call, run enableDailyLogger() in the Safari dev tools console. Then, after getting the error, save and send the logs to help@daily.co (and mention in the email to forward to me).

Meanwhile, it is not recommended to use subscriptions in p2p mode unless you can guarantee you never unsubscribe from someone once subscribed (it won't work). This is why we encourage sfu. In our own prebuilt implementation we do this by ensuring that all feeds are visible up to the number of participants that causes a switch (our recommendations and defaults switch to sfu when a 3rd person joins the call).

As for the slow join time, I'll also file to look into this. Meanwhile, you might want to try with the latest daily-js as I know we had some recent improvements there.

troutowicz commented 1 year ago

Thank you for investigating! This makes sense -- will give your feedback a try.