0

Here is a simplified sample of my Promise code:

var sharedLocalStream = null;
// ...
function getVideoStream() {
  return new Promise(function(resolve, reject) {
    if (sharedLocalStream) {
      console.log('sharedLocalStream is defined');
      resolve(sharedLocalStream);
    } else {
      console.log('sharedLocalStream is null, requesting it');
      navigator
          .mediaDevices
          .getUserMedia(constraints)
          .then(function(stream) {
            console.log('got local videostream', stream);
            sharedLocalStream = stream;
            resolve(sharedLocalStream);
          })
          .catch(reject);
    }
  });
}

I'm using this function asynchronously in several places. The issue is related to the fact that function gets called at least twice, but in a second call promise never gets resolved/rejected. This code works perfectly in Chrome. Also I tried to use Angular promises service $q, but it didn't work either.

What am I doing wrong and how to make this code work?

Also, I was thinking a lot about ways how I can avoid promises in this case and I have no choice because I forced to wait when user confirms mic&camera access request.

Update:

var constraints = {
    audio: true,
    video: true
  };
georgeawg
  • 48,608
  • 13
  • 72
  • 95
zoonman
  • 1,116
  • 1
  • 12
  • 30
  • This is a [promise anti-pattern](https://github.com/petkaantonov/bluebird/wiki/Promise-anti-patterns) because you are wrapping an existing promise in another promise rather than just returning the promise you already have. In your `if()`, you can just do `return Promise.resolve(sharedLocalStream)` and in your `else`, you can just return the promise you already have without creating a new one. – jfriend00 Jun 25 '17 at 04:59
  • 2
    Promises are not broken in firefox - the problem lies elsewhere – Jaromanda X Jun 25 '17 at 04:59
  • It's unclear to us what exactly your problem is, but I rather doubt it's a problem with promises in Firefox. It's more likely a problem with how your code is using promises or how some function you're calling responds when you call it twice. – jfriend00 Jun 25 '17 at 05:01
  • @jfriend00 could you, please, elaborate more on it? I just read this https://stackoverflow.com/questions/23803743/what-is-the-explicit-promise-construction-antipattern-and-how-do-i-avoid-it?rq=1 but as you might see, my code requires some work with just obtained stream. – zoonman Jun 25 '17 at 05:03
  • @jfriend00 - look at https://developer.mozilla.org/en-US/docs/Web/API/MediaDevices/getUserMedia instead – Jaromanda X Jun 25 '17 at 05:04
  • I gave you link to an article. There are other articles you can find. I also explained in my comment what you can do to avoid wrapping in an unnecessary promise. – jfriend00 Jun 25 '17 at 05:04
  • what is `constraints`? – Jaromanda X Jun 25 '17 at 05:05
  • @jfriend00 I'm using webrtc-adapter, it handles browsers differences. getUserMedia() works perfectly. Issue related to promises because it works fine in Chrome. – zoonman Jun 25 '17 at 05:05
  • promises work fine in Firefox, the issue is elsewhere – Jaromanda X Jun 25 '17 at 05:07
  • [here](https://jsfiddle.net/a36gfuLd/) is a truly simplified version of your *logic* – Jaromanda X Jun 25 '17 at 05:08
  • @JaromandaX - That is some obtuse code. Not obvious at all what it does or how it works. I sure wouldn't recommend anyone code that way. Also, it does not do the same thing the OP's code does. The OP's code always returns a promise. – jfriend00 Jun 25 '17 at 05:10
  • what? it's clear exactly what that code does - a very common pattern whose name I can't recall :p – Jaromanda X Jun 25 '17 at 05:10
  • `how I can avoid promises` - as `navigator.mediaDevices.getUserMedia` returns a promise, there is no way to avoid promises, if you think about it – Jaromanda X Jun 25 '17 at 05:13
  • @JaromandaX your code have 2 differences: 1. It is not returning a promise. 2. It will always create a new mediastream, which is I am trying to avoid. – zoonman Jun 25 '17 at 05:13
  • @zoonman you are wrong on two counts - 1. it is always returning a promise, 2. it will only create a new media stream the first time it's called – Jaromanda X Jun 25 '17 at 05:22
  • see https://jsfiddle.net/a36gfuLd/3/ - you can see it calls getUserMedia only once – Jaromanda X Jun 25 '17 at 05:29

2 Answers2

2

Your code has concurrency issues if getVideoStream() is called twice. Because there is no forced queuing or sequencing when getVideoStream() is called a second time before the first call to it has updated the sharedLocalStream variable, you can easily end up with a situation where you create two streams before both calls are started before sharedLocalStream has a value.

This is an issue with the design of the code, not with the platform you are running it on. The usual way around this is to store the promise from the first operation into the shared variable. You then test to see if there's a promise already in there. If there is, you just return that promise.

If you cache the promise instead of the stream, you can do it as simply as this:

var sharedLocalStreamPromise = null;
function getVideoStream() {
    // if we've already requested a local stream, 
    // return the promise who's fulfilled value is that stream
    if (!sharedLocalStreamPromise) {
        sharedLocalStreamPromise = navigator.mediaDevices.getUserMedia(constraints).catch(function(err) {
            // clear the promise so we don't cache a rejected promise
            sharedLocalStreamPromise = null;
            throw err;
        });
    }
    return sharedLocalStreamPromise;
}

The first call will initialize sharedLocalStreamPromise to a promise. When the second call (or any subsequent call) comes in, it will just return that same promise.

There is an edge case with this code which I'm thinking about. If the promise rejects and a second call to the same function has already occurred, it will also have the promise that rejects.

jfriend00
  • 683,504
  • 96
  • 985
  • 979
  • Looks like I have to implement queuing and wait until user confirms the request. Is there a good way to do it? – zoonman Jun 25 '17 at 05:19
  • I like your solution, I was thinking to store promise, but ended up storing a stream which was a bad idea. – zoonman Jun 25 '17 at 05:27
  • `clear the promise so we don't cache a rejected promise` in this case it makes no sense, because once a user denies getUserMedia, all subsequent calls to getUserMedia will not prompt the user again, and just return a rejected promise – Jaromanda X Jun 25 '17 at 05:35
  • (unnecessary) globals suck, by the way :p – Jaromanda X Jun 25 '17 at 05:37
  • @JaromandaX - It can be wrapped in an IIFE if the OP wants. Would absolutely do that if this was a library being shared with others. More optional if just your own project. – jfriend00 Jun 25 '17 at 05:41
  • @JaromandaX I don't understand your code. It is unreadable and it has way too many brackets so I'm loosing context. And it is about 1:42 AM. – zoonman Jun 25 '17 at 05:42
  • fair enough @zoonman - IIFE's do add brackets, don't they - my code was an attempt to avoid unnecessary globals :p – Jaromanda X Jun 25 '17 at 05:43
  • They do, @JaromandaX. Anyway, thank you too. You gave me a kick to start reading new JS specs ;-) – zoonman Jun 25 '17 at 05:50
0

Without IIFE

let sharedLocalStreamPromise;

function getVideoStream() {
    return sharedLocalStreamPromise = sharedLocalStreamPromise || navigator.mediaDevices.getUserMedia(constraints);
};

with IIFE

const getVideoStream = (() => {
    let sharedLocalStreamPromise;
    return () => sharedLocalStreamPromise = sharedLocalStreamPromise || navigator.mediaDevices.getUserMedia(constraints);
})();

Alterntively

var getVideoStream = () => {
    const result = navigator.mediaDevices.getUserMedia(constraints);
    getVideoStream = () => result;
    return getVideoStream();
};

this last one creates a result (being the promise returned by getUserMedia) the first time it is called, and then overwrites itself to just return that result ... no conditionals in subsequent invocations of getVideoStream - so, theoretically "faster" (in this case, speed is a moot point though)

Jaromanda X
  • 53,868
  • 5
  • 73
  • 87