removeTrack: throw exception if sender is not in connection's set of senders

[mparis]

Hello,
I think that if removeTrack's sender param is not in connection's set of senders and exception should be thrown.
This is quite useful for developers to find problems in their apps.

Imagine that an app has 2 PeerConnections and call pc1.removeTrack (senderOfPc2).
If an exception is thrown, it would help the developer to see the problem and fix it.

Refs
[1] http://w3c.github.io/webrtc-pc/#dom-rtcpeerconnection-removetrack

[adam-be]

In all versions of removeTrack/Stream, this situation has been a no-op. The same goes for, e.g., RTCPeerConnection.close (it just returns if the pc is closed already). That way they can be safely called without checking some state or property beforehand. This approach has drawbacks, as you point out above, but I don't think we can start throwing at this time as it could make existing apps throw unexpectedly.

[mparis]

Hello @adam-be,
we shouldn't compare the case I told to RTCPeerConnection.close, which can be seen as an idempotent method.

we can start throwing at this time as it could make existing apps throw unexpectedly.
In relation to that, we are changing a lot of things in the spec that can cause this, but it is normal because the spec is under development and there is not a release yet.

So, I would consider throwing an exception in the case I mentioned.
I can tell you that some developers using our implementation have committed this error (confusing the local MediaStreamTracks/RtpSenders of different peerconnections) and the exception helped them a lot ;).

[jan-ivar]

@mparis removeTrack is also idempotent. Consider:

var sender = pc.addTrack(track, stream);
pc.removeTrack(sender); // fine
pc.removeTrack(sender); // fine

This is consistent with other methods as well:

var track = stream.getTracks()[0];
stream.removeTrack(track); // fine
stream.removeTrack(track); // fine

Idempotency simplifies, especially in the latter case, as a track may be in multiple streams at once.

I don't think we want to lose that property, but maybe we could distinguish the above from your case:

var sender = pc1.addTrack(track, stream);
pc2.removeTrack(sender);

Doing so would require implementations to know that this sender was not born of this peer connection, which is always a pilot error. This seems doable and worthwhile to me.

[jan-ivar]

The key distinction being that a sender can never be in another peer connection.

[taylor-b]

I like the idea of differentiating whether or not the sender was created by the peer connection.

In one case, the argument to removeTrack would have been valid, but the method call is superfluous since it was already called. So it makes sense for it to be a no-op.

But in the other case, the argument to removeTrack would never have been valid. So I can see the argument for throwing an exception.

[adam-be]

So, I would consider throwing an exception in the case I mentioned.
I can tell you that some developers using our implementation have committed this error (confusing the local MediaStreamTracks/RtpSenders of different peerconnections) and the exception helped them a lot ;).

I agree that this makes sense.

[alvestrand]

Fixed by #844