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

RTCPeerConnection: Write tests for #setLocalDescription #7

Open
aisouard opened this issue Mar 21, 2017 · 4 comments
Open

RTCPeerConnection: Write tests for #setLocalDescription #7

aisouard opened this issue Mar 21, 2017 · 4 comments
Assignees
Milestone

Comments

@aisouard
Copy link
Owner

No description provided.

@aisouard aisouard added this to the 0.0.1 milestone Mar 21, 2017
@aisouard aisouard self-assigned this Mar 31, 2017
@aisouard
Copy link
Owner Author

Working under branch set-local-description

@clemos
Copy link
Contributor

clemos commented Apr 1, 2017

FYI, I had setLocalDescription working on #5.
I can understand that you want to start clean though ;)

@aisouard
Copy link
Owner Author

aisouard commented Apr 3, 2017

I really prefer to make sure that the tests are working according to the spec, your work is helpful for the events, I'm getting stuck right now because the onsignalingstatechange gets reset to null (on my side), not sure why, I'll find out this week.

The new specs looks like bad news by the way:

    Promise<void> setLocalDescription(RTCSessionDescriptionInit description,
                                      VoidFunction successCallback,
                                      RTCPeerConnectionErrorCallback failureCallback);

Run the steps specified by RTCPeerConnection 's setLocalDescription method with
description as the sole argument, and let p be the resulting promise.

Upon fulfillment of p, invoke successCallback with undefined as the argument.

Upon rejection of p with reason r, invoke failureCallback with r as the argument.

Return a promise resolved with undefined.

Callbacks arguments are mandatory.
Why should I return a promise and set a callback at the same time ?
Does it destroy the purpose of promises ?
Or is it just for backward-compatibility, since callbacks were removed inside the November 2016 spec ?

@clemos
Copy link
Contributor

clemos commented Apr 3, 2017

It's a bit weird, but I think it somehow makes sense:

  • callbacks are mandatory to enforce definition of at least one success / error behaviour (which is difficult to do with promises)
  • a promise is returned in all cases, which is always better than returning null (and maybe to comply to this spec). The user still has the choice to drop it if he doesn't need it.

I think this implies that the callbacks must be handled using the promise internally, in order to avoid Uncaught error (in promise) if the promise is not used / if no .catch is defined.

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

No branches or pull requests

2 participants