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

On 2013-06-04 17:52, Adam Roach wrote:
> On 6/4/13 07:52, Adam Bergkvist wrote:
>> On 2013-06-01 02:04, Justin Uberti wrote:
>>> Was a decision reached here? It seems unfortunate to have to add
>>> success/failure callbacks to several methods just to better handle being
>>> called in the "closed" state.
>> No, I don't think that would be nice.
> Anything that can be queued needs to handle async failures, which means
> that all queueable actions need failure callbacks.

True and I didn't interpret Justin's statement as arguing against that 

>> The spec is a bit inconsistent at the moment but I think that
>> setLocal/RemoteDescription handles this fairly well. The state is
>> checked for closed before the real work is handed over to the WebRTC
>> backend. That makes these methods consistent with other methods (e.g.
>> addStream) that doesn't have success/failure callbacks; they don't do
>> anything on a closed PeerConnection.
> The thing here is that we've identified situations in which things like
> addStream needs to be able to return errors in an asynchronous fashion
> also. Most of these are internal errors of one kind or another, but some
> of them can result from resource exhaustion.
> I'm also not certain we have the list of queueable event correct yet.
> The ordering of, say, addStream versus createOffer is clearly important.
> The fact that one can be queued behind other operations while the other
> cannot will cause application developers more than a little surprise.
> The same goes for addIceCandidate.
> In practice, by the time we have this fully implemented, I suspect we'll
> find reasons to queue most or all operations on RTCPeerConnection.
> That said, I agree with the general *principle* that we don't want to
> add failure callbacks to methods for the exclusive purpose of reporting
> a closed condition. I just don't think we have that many places where we
> don't need failure callbacks for other reasons as well. Let's look at
> the current operations:
> createOffer - has a failure callback
> createAnswer - has a failure callback
> setLocalDescription - has a failure callback
> setRemoteDescription - has a failure callback
> updateIce - We have not implemented this yet. I suspect, when we get
> around to doing so, we'll find that it needs to be async and allow error
> callbacks for reasons other than signling state.
> addIceCandidate - Per conversation back in January, really needs a
> failure callback.
> getLocalStreams - See below
> getRemoteStreams - See below
> getStreamById - See below
> addStream - Needs a failure callback for resource exhaustion purposes
> removeStream - As I argue above, since this can change the behavior of
> the operations we already know need to be queued, we'll eventually
> determine that it needs queueing also -- which means callbacks.

I remember you started a thread on this [1], but you never commented on 
the replies.

> close - We've agreed that close() on a closed connection generates
> neither an error nor an exception

Yes. If no-one objects, I think that should go into the next editor's draft.

> For getLocalStreams, getRemoteStreams, and getStreamById, I think we
> need to handle these the same way we treat attempts to read
> localDescription and remoteDescription on a closed connection. A naive
> reading of the spec right now indicates that reading localDescription on
> a closed connection returns whatever the localDescription was when it
> was closed (since there is no special handling described for reading
> these attributes on a closed connection). If this is what we want, the
> spec should make such behavior explicit. If we want something different
> (e.g., throwing an exception or returning a null), then we need to add
> text to the document to clarify such behavior.

Yes, the current behavior is to return whatever was present when the 
PeerConnection closed down. We used to have an explicit note about that 
(back when we had the localStreams and remoteStreams attributes), but it 
was lost somehow. I can take an action to add similar text again.



Received on Wednesday, 5 June 2013 09:53:12 UTC