search

ionorg / ion-sdk-js

ion javascript sdk
MIT License
102 stars 70 forks source link

Resolve `join` promise after API channel ready #185

Closed kevinmcconnell closed 3 years ago

kevinmcconnell commented 3 years ago

Previously, the (implicit) promise returned by join would resolve after establishing the transports, but before the API data channel was attached. This meant there was a small window of time where the API data channel was not ready, and trying to use the subscriber API during that time would fail silently.

This change returns a promise that does not resolve until after the API channel is established.

kevinmcconnell commented 3 years ago

@billylindeman here's the replacement for #182. This way definitely turned out nicer -- thanks for the suggestion!

tarrencev commented 3 years ago

awesome thanks!

adwpc commented 3 years ago

Im so sad This change make a error in ion-app-web: trickle error: no rtc transport exists for this Peer

It worked fine when i use v1.7.0

adwpc commented 3 years ago

@kevinmcconnell @tarrencev I will revert this until find a good way

kevinmcconnell commented 3 years ago

@adwpc thanks for letting me know. It looks like the problem comes from a combination of this change + the new Ion connector. It results in a race condition because the callbacks can fire before connector.sfu is set.

I'll open a new PR with a fix.

kevinmcconnell commented 3 years ago

@adwpc I've opened #192 with a fix for this, and I've tested to make sure it now works with ion-app-web.

Would you mind taking a look to make sure it works for you? Thanks!

adwpc commented 3 years ago

@kevinmcconnell Thanks! I made a pr too. I tested yours, it worked https://github.com/pion/ion-sdk-js/pull/194

manishiitg commented 3 years ago

Still facing the same issue. the issue is not resolved for now

manishiitg commented 3 years ago

https://github.com/pion/ion-sdk-js/pull/194

this should fix the issue but it was closed instead of getting merged?

kevinmcconnell commented 3 years ago

@manishiitg After #192, I no longer see this error. You are still seeing it after that? Can you tell me how to reproduce it?

manishiitg commented 3 years ago

@kevinmcconnell https://github.com/pion/ion-app-web/blob/master/src/Conference.jsx#L108 here. when publish is called its called after .then() so its get some time for _sfu to get set

if lets say if you call connector.sfu.publish({....}); or 

const localStream = new LocalStream(mystream, {
                            resolution: 'hd',
                            codec: 'vp8',
                            audio: true,
                            video: true,
                            simulcast: false,
                        })
connector.sfu.publish(localStream);

directly at line no https://github.com/pion/ion-app-web/blob/master/src/Conference.jsx#L101 u will get this error