fix: e2ee setup hardening and tests - #2044
Conversation
🦋 Changeset detectedLatest commit: 59e774e The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
size-limit report 📦
|
0469935 to
aec3fdc
Compare
1egoman
left a comment
There was a problem hiding this comment.
Generally I can follow what's going on, but I've yet to dig really deep into e2ee so my review is fairly high level
| // desync our bookkeeping from reality -- which previously let a later | ||
| // resubscribe skip setup while frames kept flowing through a cryptor with no | ||
| // participant assigned. | ||
| clearTimeout(this.undecryptedTrackTimeout); |
There was a problem hiding this comment.
nitpick: Maybe clear this.undecryptedTrackTimeout explictly here so there's not a no longer active timeout id stored? That could be a source of more bugs in the future.
There's another place in scheduleUndecryptedTrackWatchdog where I think it would be good to clear it too, before the if (operation !== 'decode') { early bail out.
| // @ts-ignore | ||
| previousTrackId: receiver[E2EE_TRACK_ID], | ||
| codec, | ||
| participantIdentity, | ||
| hasPacketTrailer, | ||
| }, | ||
| }; | ||
| this.worker.postMessage(msg); | ||
| // @ts-ignore | ||
| receiver[E2EE_TRACK_ID] = trackId; |
There was a problem hiding this comment.
question: What's up with these @ts-ignores? Is it worth making the if condition earlier on if (E2EE_FLAG in receiver && E2EE_TRACK_ID in receiver) { to drop them?
Follow up to #2043
Fixes E2EE subscribers rendering a permanently black tile with nothing logged.
Three independent defects, each of which could leave a subscribed, encrypted track with no live decrypt transform:
Also removes the now-dead isReuse flag and hardens pipeThrough against synchronous throws.