Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Need to specify what happens if createDataChannel is called with an invalid ID #746

Closed
taylor-b opened this issue Aug 17, 2016 · 9 comments

Comments

@taylor-b
Copy link
Contributor

Currently, step 10 of createDataChannel instructs:

if the value of the id attribute is taken by an existing RTCDataChannel , throw a ResourceInUse exception and abort these steps.

However, there are situations where an id is unusable even though no existing RTCDataChannel with that ID exists. One unavoidable situation is when the requested ID is 65535 (the maximum value of a WebIDL unsigned short). This is also the maximum number of SCTP streams, which means the highest valid stream ID is 65534, since they're 0-based.

It's also possible for a WebRTC implementation to negotiate less than the maximum number of streams. Suppose 1000 streams are negotiated, and an application tries to create a data channel with stream ID 2000. When this happens, should an error be surfaced, or should the WebRTC implementation attempt to add 1001 additional streams using a RE-CONFIG message? If we prefer the latter, this behavior should probably be detailed in the rtcweb data channel spec.

@adam-be
Copy link
Member

adam-be commented Aug 24, 2016

In the first case I think we can throw an OperationError. In WebIDL there is an IndexSizeError ("The index is not in the allowed range") that we could use, but it might be safer to just go with OperationError.

The second issue should perhaps be taken to list to get the WG's view.

@taylor-b
Copy link
Contributor Author

I just listened to the discussion of this topic by the rtcweb working group at IETF 88. They definitely decided against doing the RE-CONFIG, because of the added complexity. The conclusion was to just negotiate the maximum number of streams. There was some concern that current SCTP implementations allocate memory for each stream (this includes usrsctp, used by Chrome as of typing this). But people argued that implementations should be able to avoid this by using sparse data structures.

However, negotiating the maximum number of streams is still only a "SHOULD", not a "MUST", so the case still needs to be addressed.

My proposal would be to invoke the onerror event handler whenever this condition occurs. Why not throw an exception from createDataChannel? Because createDataChannel can be called before the PC knows how many streams will be negotiated, so the error needs to occur asynchronously in that case. And it would be confusing for developers if the condition sometimes resulted in onerror, and sometimes resulted in a DOMException, depending on the timing of the SCTP handshake.

taylor-b added a commit to taylor-b/webrtc-pc that referenced this issue Oct 1, 2016
Fixes issue w3c#746.

For an ID of 65535, createDataChannel will throw an exception. If
it's determined that an ID is invalid for any other reason (such as
shortage of negotiated SCTP streams), or if no more valid IDs are
available when the user agent tries to pick an ID, the data channel will
fire an "error" event and be closed.

For now, I'm leaving the actual error as a "NetworkError", since I
think we need to separately decide how error events on data channels
work (see issue w3c#846).
@alvestrand
Copy link
Contributor

The 65535 issue was addressed by #847, but "didn't negotiate them all" seems to be still open. @taylor-b , comments?

@taylor-b
Copy link
Contributor Author

taylor-b commented Oct 6, 2016

I thought this paragraph would cover that case:

In some cases, the user agent may be unable to create an RTCDataChannel's underlying data transport. For example, the data channel's id may be outside the range negotiated by the [RTCWEB-DATA] implementations, or in the case where the user agent generates an id according to [RTCWEB-DATA-PROTOCOL], no more IDs may be available. When the user agent determines that an RTCDataChannel's underlying data transport cannot be created, the user agent must queue a task to run the following steps:

"Didn't negotiate an ID for a data channel" would fall in the bucket of "underlying data transport cannot be created". At least, if I understand the definition of "underlying data transport" correctly.

@alvestrand
Copy link
Contributor

(sorry for dropping the ball)
the question was "if we negotiate a smaller range, what will happen?"
ie
createDataChannel(id=50000)
<negotiation happens, remote side says "valid range 1-1000">
then what?

If it can't happen, that's OK. (but it's already happened.)

@taylor-b
Copy link
Contributor Author

The intent is that after the valid range is negotiated, and it's not big enough for an RTCDataChannel, it fires an error and goes to "closed". This is what I believe I suggested a couple interims ago, and the group agreed to.

But if that's not clear from how this PR is written, I'm open to suggestions. I tried to keep with the policy of not getting too deep into SCTP specifics, and instead consider this a variant of "failed to establish underlying data transport" (see stuff quoted above)

@alvestrand
Copy link
Contributor

If we have a requirement that SCTP negotiation has to negotiate all channels from 0...65535, that's fine. This needs to go into the IETF documents somewhere. Which one?
(If we have a bug filed against an IETF document and referenced here, I think we can close this issue.)

@taylor-b
Copy link
Contributor Author

In the IETF data channel spec section 6.2, it's a SHOULD, not a MUST. I wasn't proposing we change that, just that we fire an error if the negotiated range is smaller than the ID used by a data channel.

@alvestrand
Copy link
Contributor

OK, I believe we have it OK now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants