1272688 - Talky.io broken by landing bug 998546 (ontrack and onaddstream fire too late)

Status: RESOLVED WORKSFORME

(Core :: WebRTC, defect, P1)

Description

[Randell Jesup [:jesup] (needinfo me)]

+++ This bug was initially created as a clone of Bug #998546 +++

Talky.io is broken in Nightly; likely due to landing this bug.  It's very possible (likely) that this is a bug in talky's JS code, but filing a bug to track the issue.

Comment 1

[phil]

I've not yet fixed this, but I've made a start looking into it. It appears that talky binds to onstreamadded events fairly late. I'm assuming that previously firefox emitted those events late enough on streams that are there at the start of the peer connection, that they were still (mostly/always?) emitted after we bind to them; but now that the events are emitted earlier, they're being emitted after we've actually bound to the event, so we miss them.

Looks like we just need to bind earlier, or iterate over existing streams when we initialize things.

Comment 2

[phil]

Okay, verified that it was an issue on our side. Fixed and verified on our staging server, and will be fixed in our next release. Thanks for the heads up!

Comment 3

[phil]

(In reply to phil from comment #2)
> Okay, verified that it was an issue on our side. Fixed and verified on our
> staging server, and will be fixed in our next release. Thanks for the heads
> up!

Deployed now, confirmed that talky.io is fixed on nightly

Comment 4

[Daniel Holbert [:dholbert]]

(In reply to phil from comment #3)
> Deployed now, confirmed that talky.io is fixed on nightly

Are you sure? I tried & failed to have a meeting via Talky.io today in Nightly, and it worked when both parties switched to Firefox release.  (In Nightly, it behaved as if the other person simply hadn't connected yet.  Also: when I was using Nightly and the other person was using Release, he could see/hear me, but Talky.io told me (in Nightly) that there was no one else connected.)

Comment 5

[Daniel Holbert [:dholbert]]

(I can file a new bug for my issue described in comment 4, too, but given the timing, I'm suspecting that it's a version of this same bug.)

Comment 6

[phil]

Oh dang, sorry forgot to come back and update this. We did fix it, but our fix had problems in chrome so we rolled back.

Bugfix is still on stage.talky.io if you want to verify. Will get it re-fixed in production next week. Thanks!

Comment 7

[Daniel Holbert [:dholbert]]

Yup, https://stage.talky.io/ does indeed work in Nightly. Thanks!

Comment 8

[Maire Reavy [:mreavy]]

I'm going to assign this to myself to make sure the talky.io fix rolls out and stays rolled out. 

Phil -- Were the problems in chrome problems with chrome release or chrome beta/canary?

Comment 9

[phil]

@mreavy sorry, it was just a logic bug in our fix, that was exposed in chrome. Will definitely get it rolled out monday if not over the weekend.

Comment 10

[phil]

Okay, redeployed to talky.io and looks good, sorry about the delay.

Please note that it looks like our asset caching might be a little too optimistic, so be sure you hard-refresh to get the latest version.

Comment 11

[Maire Reavy [:mreavy]]

(In reply to phil from comment #10)
> Okay, redeployed to talky.io and looks good, sorry about the delay.
> 
> Please note that it looks like our asset caching might be a little too
> optimistic, so be sure you hard-refresh to get the latest version.

How optimistic?  I.e. how long will existing (Nightly 49) users who don't know to hard-reset be broken?

Comment 12

[phil]

(In reply to Maire Reavy [:mreavy] (On PTO until July 5th) from comment #11)
> (In reply to phil from comment #10)
> > Okay, redeployed to talky.io and looks good, sorry about the delay.
> > 
> > Please note that it looks like our asset caching might be a little too
> > optimistic, so be sure you hard-refresh to get the latest version.
> 
> How optimistic?  I.e. how long will existing (Nightly 49) users who don't
> know to hard-reset be broken?

mreavy email me to ask about this and I replied to them there, but forgot to update this issue: I added a cache-buster to our html to enforce the new version for everyone, so I think we're good to close this now afaik.