Warning: .then() only accepts functions but was passed

iaptsiauri commented 7 years ago

I the following warning Warning: .then() only accepts functions but was passed: [object Undefined], [object Undefined], Once when executing Video.createLocalTracks(), and 5 times when remote tracks are added. This behaviour only happens when I include the twilio-video as following import * as Video from 'twilio-video'; but the warning is never shown if I use the CDN.

markandrus commented 7 years ago

Hi @iaptsiauri,

Thanks for writing in. In the future, can you please use the issue template? My team put a lot of thought into the information we ask for in the issue template, and in order to help debug your issue I'm going to have to re-ask you for that same information. Specifically, can you tell me

What browser(s) you are testing in
What other libraries you may be using (e.g. Babel, anything affecting Promise behavior, etc.)
How to reproduce the issue—a minimal code sample would be ideal

Thanks, Mark

markandrus commented 7 years ago

Hi @iaptsiauri,

Are you by any chance using Bluebird? I see the error in their src/promise.js.

I'll try to repro by loading Bluebird alongside twilio-video.js, but if you can provide a repro example, that would be very helpful.

Thanks, Mark

iaptsiauri commented 7 years ago

@markandrus First of all I am really sorry for not providing enough information to you guys, I was just super confused when encountered the error.

I see the warning to appear in Chrome, Opera and Firefox.
We are using Aurelia framework which uses Bluebird.
I will provide the sample app asap at the moment it is quite late over here :)

Thanks

iaptsiauri commented 7 years ago

Hi @markandrus, This is the url for the reproduction example repro example. The project is built with Aurelia Framework, and uses Aurelia CLI.

Additionally what I have noticed is the following, Whenever iceServers are not specified Video.connect() throws the following exception. This exception can be reproduced by omitting the iceServers in the example

TypeError: Promise.race is not a function
    at poll (http://localhost:3000/scripts/video-call-bundle.js:89:15145)
    at NTSIceServerSource.start (http://localhost:3000/scripts/video-call-bundle.js:89:13901)
    at createRoomSignaling (http://localhost:3000/scripts/video-call-bundle.js:89:5240)
    at getLocalTracksSucceeded (http://localhost:3000/scripts/video-call-bundle.js:89:1035)
    at getLocalTracksSucceeded (http://localhost:3000/scripts/video-call-bundle.js:89:6111)
    at <anonymous>

markandrus commented 7 years ago

Thanks for following up with more details and a repro example! I will take a look in more detail on Monday. I don't know much about Aurelia, so I'll have to look into it. A quick Google search does show some issues with Promise.race: http://stackoverflow.com/questions/39128930/aureliajs-where-is-promise-race I personally haven't used Bluebird, so I'm not sure if they provide this function or not.

iaptsiauri commented 7 years ago

Hi @markandrus, I see you have quite a lot going on at the moment, :) can you please provide me with the some starting info, maybe I can help you out and figure out why the warning is fired

markandrus commented 7 years ago

Hi @iaptsiauri,

OK, so I did a deep-dive here, and I've tracked down the warning in both your reproduction app and in a separate JSFiddle. Here's the full stack trace:

bluebird.js:1542 Warning: .then() only accepts functions but was passed: [object Undefined], [object Undefined]
    at IncomingRequest.reply (https://media.twiliocdn.com/sdk/js/video/releases/1.0.0/twilio-video.js:14618:59)
    at InviteClientContext.onInfo (https://media.twiliocdn.com/sdk/js/video/releases/1.0.0/twilio-video.js:5507:15)
    at InviteClientContext.receiveRequest (https://media.twiliocdn.com/sdk/js/video/releases/1.0.0/twilio-video.js:15655:25)
    at InviteClientContext.receiveRequest (https://media.twiliocdn.com/sdk/js/video/releases/1.0.0/twilio-video.js:17232:45)
    at Dialog.receiveRequest (https://media.twiliocdn.com/sdk/js/video/releases/1.0.0/twilio-video.js:11249:16)
    at UA.receiveRequest (https://media.twiliocdn.com/sdk/js/video/releases/1.0.0/twilio-video.js:19608:14)
    at Transport.onMessage (https://media.twiliocdn.com/sdk/js/video/releases/1.0.0/twilio-video.js:18833:17)
    at WebSocket.ws.onmessage (https://media.twiliocdn.com/sdk/js/video/releases/1.0.0/twilio-video.js:18701:17)

The reason this warning is generated is because one of our dependencies, SIP.js, depends on the ability to omit the onFulfilled and onRejected handlers from Promise.prototype.then. According to MDN, this is a valid usage:

Note: If both arguments are omitted, or are provided non-functions, a new Promise is created with no additional handlers, simply adopting the final state of the Promise that then is called on.

Testing in Chrome's JavaScript console confirms this:

However, Bluebird has implemented some different behavior. See here:

The warning is ok and expected as it's the Promises/A+ test suite testing behavior when .then is incorrectly called with non function values. In practice this warning helps people new to promises quickly see where they made a mistake.

Although I'm not sure what the mention to "expected" means—I find no reference to expecting a warning in either lib/tests/2.2.1.js, lib/tests/2.2.2.js, or lib/tests/2.2.3.js of promises-aplus/promises-tests. And this warning behavior has not been implemented in the browsers.

Luckily, this warning behavior can be worked around in Bluebird, by calling a non-standard Promise.config API they provide. Here's another JSFiddle demonstrating. Without the Promise.config call, Bluebird will log the warning:

if (typeof Promise.config === 'function') {
  Promise.config({
    warnings: false
  })
}

Promise.resolve('hello world').then(undefined, undefined)

So, I'd recommend using this Promise.config API to suppress the warning. I'll still need to look into the Promise.race issue.

markandrus commented 7 years ago

@iaptsiauri, so here's the Promise.race issue. I was quite confused, because Promise.race is straight up undefined while Promise.all was present and the non-standard Promise.version indicated a recent build of Bluebird (3.5.0). Then I looked at vendor-bundle.js, and I saw this:

/**
 * bluebird build version 3.5.0
 * Features enabled: core
 * Features disabled: race, call_get, generators, map, nodeify, promisify, props, reduce, settle, some, using, timers, filter, any, each
*/

So apparently whatever Bluebird build that Aurelia is using has Promise.race explicitly disabled, which is... strange, IMO, considering this is a standard API. After some poking around, it looks like, if you make this change, the property will be set:

index 624e33e..3de53e4 100644
--- a/aurelia_project/aurelia.json
+++ b/aurelia_project/aurelia.json
@@ -91,7 +91,7 @@
         "name": "vendor-bundle.js",
         "prepend": [
           "node_modules/babel-polyfill/dist/polyfill.js",
-          "node_modules/bluebird/js/browser/bluebird.core.js",
+          "node_modules/bluebird/js/browser/bluebird.js",
           "node_modules/requirejs/require.js"
         ],
         "dependencies": [

Maybe this is all obvious if you know Bluebird, but it's totally foreign to me 😅 Can you try these changes out?

Best, Mark

iaptsiauri commented 7 years ago

Hi @markandrus, Thank you very much for the clarification. Suppressing the warning is good enough and promise race got resolved as well

twilio / twilio-video.js

Warning: .then() only accepts functions but was passed #107