Re: Operations in invalid states: Exceptions don't make sense.

On 2013-05-22 09:31, Adam Roach wrote:
> Currently, the WebRTC spec has some implication that a PeerConnection's
> signalingState should be checked at the time an operation (e.g.,
> setRemoteDescription) is enqueued, and an that an exception should be
> thrown if the state is not appropriate for the indicated operation /at
> that time//,/ regardless of how the already-enqueued operations may
> change that state.
>
> This is covered in section 4.6.1 (Error Handling / General Principles):
> " An exception /MUST/ be thrown... [if a] function call was made when
> the RTCPeerConnection is in an invalid state, or a state in which that
> particular function is not allowed to be executed."
>
> This interacts badly with the conceptual model that we currently have
> around operations, which is that scripts are allowed to make several
> calls on the PeerConnection without first waiting for the prior one to
> complete (with the result being that the PeerConnection queues such
> operations until the previous one completes).
>
> Roughly speaking, this means that the PC state can be okay at the time
> an operation is enqueued, but invalid by the time the operation is
> executed. Conversely, the PC state may be invalid at the time an
> operation is attempted, but be changed to an invalid state by the time
> the operation is to be run.
>
> The first set of circumstances requires us to allow async error
> callbacks for invalid states.
>
> The second set of circumstances means that synchronous checks will,
> under some circumstances, throw exceptions when it is inappropriate to
> do so.
>
> In other words, I think this points to an absolute requirement to check
> states async and an absolute prohibition on checking them synchronously.
>
> The specific change I'm requesting is:
>
>   * Remove the second bullet from section 4.6.1
>   * Change step 1 of "addStream" in section 4.3.2.2 to end with
>     "...abort these steps, and call the addStream error callback with an
>     RTCError of  INVALID_STATE"
>   * Remove step 1 from "close" in section 4.3.2.2
>   * Change step 2 of "addStream" in section 4.3.2.2 to end with
>     "...abort these steps, and call the removeStream error callback with
>     an RTCError of  INVALID_STATE"
>   * Change the first bullet of "setLocalDescription" in section 4.3.2.2
>     to end with "...abort these steps, and call the setLocalDescription
>     error callback with an RTCError of INVALID_STATE"
>   * Change step 1 of section 5.1.2 to end with "...call the
>     createDataChannel error callback with an RTCError of INVALID_STATE."
>   * Remove step 2 from section 5.2.3 (for this reason and for the reason
>     that readyState no longer exists)
>   * Change step 1 of section 7.2.1 to end with "...call the getStats
>     error callback with an RTCError of INVALID_STATE."
>   * Change step 1 of section 8.2.2, setIdentityProvider to end with
>     "...abort these steps, and call the setIdentityProvider error
>     callback with an RTCError of  INVALID_STATE"


Thanks for bringing this up; there's some text that hasn't been properly 
updated WRT the operation queuing.

Not all functions you mention above are candidates to be queued. The 
spec lists createOffer, setLocalDescription, createAnswer and 
setRemoteDescription [1]. It may be that this list should be updated.

For the calls that may be queued, we have the option to split them up 
into a section that first runs on the main loop and checks if the 
PeerConnection isn't closed, and then adds a task to the queue (that 
also may need to check the state when executed). The first check would 
be valid since the PeerConnection can't recover from the closed state. 
This would be more consistent with non-queueable operations and would 
avoid queuing operations on a closed PeerConnection.

/Adam

[1] http://dev.w3.org/2011/webrtc/editor/webrtc.html#operation

Received on Thursday, 23 May 2013 09:14:18 UTC