Re: Feedback on Pull Request - RE: Session cancellation and reconnection

Hi MarkFo,

thanks for the detailed reply - looks like we're on the same page.

On Wed, 2014-09-17 at 17:26 -0700, mark a. foltz wrote:
 
> On Wed, Sep 17, 2014 at 12:45 PM, Rottsches, Dominik
<dominik.rottsches@intel.com> wrote:


> >         Would it be more consistent to just return a session object,
> >         for which we could define an additional - for example -
> >         "Idle" state, or use the "Disconnected" state as well, then
> >         transition that to "Connected". [...]

> It is simpler, but I could see some drawbacks.
> 
> 
> First, there was some feedback from Jonas and Anton that returning a
> Session from a call to joinSession() for a presentation that doesn't
> exist was counterintuitive, and a Promise that resolves to a valid
> Session or rejects was more ergonomic.  Perhaps we could introduce an
> "unknown" state for this case, unless we don't want to leak the
> existence or non-existence of a session.

It might be considered somewhat counterintuitive to return a session
that's not usable yet, yes. I think with a descriptive pre-ready state,
this might not feel too strange. At least we would resolve having to
track the Promise result and event state in parallel, and the redundant
representation of state in two places.
> 
> Also, the page would need to be careful to avoid a race condition
> where:
> 
> 
> (1) It calls joinSession(url), which immediately returns a
> disconnected session
> (2) Since the page doesn't have a connected session, it offers the
> show() button
> (3) User clicks show() and page calls startSession(url) to generate a
> new (disconnected?) session
> (4) Session in #1 fires onstatechange and becomes connected.  Now page
> has two sessions.  Does onstatechange fire for session in #3 as well?

If I am not mistaken, I see a similar race condition in the promise case
and the example code in your pull request. Let's say, joining and
existing session takes longer than the availability notification, then
the "show" button becomes active and clickable. Assuming the user clicks
on it before it gets disabled by the joinSession promise getting
resolved, we now have a call to startSession pending, which will
override the previously joined session once it completes.

> We could mandate that the sessions returned in #1 and #3 end up being
> the same objects so the page could detect this scenario and the event
> handler only gets fired once.  I think working through a code sample
> with the non-Promise API would probably clear this up :)

Yes. So it looks like, when using the onstatechange handler approach, we
need to clearly signal the case where joining fails. That could be for
example a transition from "unknown"/"idle"/"connecting" to
"disconnected". Then the page could wait until the result of joining is
clear, before showing a startSession() UI.
> 
> Would it make sense for you fork my pull request and update it in a
> separate branch, so we could refer to the two APIs and code samples
> (Promises and non-Promises)?

Yes, I'll do that in a separate branch. Since I am a little busy
preparing for a workshop, I might not be able to finish that today, but
hopefully tomorrow, so that we have another draft to discuss.

Dominik

Received on Thursday, 18 September 2014 15:58:37 UTC