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

Multiple 'then' in Promises #894

Closed
gfwilliams opened this issue Aug 7, 2016 · 5 comments
Closed

Multiple 'then' in Promises #894

gfwilliams opened this issue Aug 7, 2016 · 5 comments

Comments

@gfwilliams
Copy link
Member

Looks like there's a problem:

http://forum.espruino.com/conversations/290967/

Can't believe I didn't spot/test this at the time, but should be an easy fix.

@radist2s
Copy link

radist2s commented Jul 4, 2018

@gfwilliams, I'm not sure problem is completely fixed.

function onInit() {
  var promise = new Promise(function (resolve) {
    setTimeout(resolve, 1000);
  });

  promise.then(function () {
    console.log('first');

    promise.then(function () {
      console.log('third');
    });
  });

  promise.then(function () {
    console.log('second');
  });

  setTimeout(function () {
    console.log('ready for fourth promise');

    promise.then(function () {
      console.log('fourth');
    });
  }, 1500);
}

save(); // just hack

Will output:

first
second
third
ready for fourth promise

Fourth callback will never called.

Few more cases.

function onInit() {
  var promise = new Promise(function (resolve) {
    setTimeout(resolve, 1000);
  });

  promise.then(function () {
    console.log('first');

    promise.then(function () {
      console.log('third');
    });
  });

  /*promise.then(function () {
    console.log('second');
  });*/

  setTimeout(function () {
    console.log('ready for fourth promise');

    promise.then(function () {
      console.log('fourth');
    });
  }, 1500);
}

save();

And output:

first
ready for fourth promise

Third callback will never called, same as fourth.
But this example works properly:

function onInit() {
  var promise = new Promise(function (resolve) {
    setTimeout(resolve, 1000);
  });

  promise.then(function () {
    console.log('first');

    promise.then(function () {
      console.log('third');
    });
  });

  promise.then(function () {
    console.log('second');
  });

  setTimeout(function () {
    console.log('ready for fourth promise');

    promise.then(function () {
      console.log('fourth');
    });
  }, 500); // decreased timeout for chaining with promise before it resolves
}

save();

Output will be correct:

ready for fourth promise
first
second
fourth
third

upd: firmware 1v99, esp32

@gfwilliams
Copy link
Member Author

Thanks, I created an issue for it ^^^

Basically this is when you call then on a promise that has already been resolved. I didn't even realize that was allowed.

If you want to work around it, just add all the then and catch you need to it before you resolve it.

Did you actually hit this in real-life code, or were you running through some tests?

@radist2s
Copy link

radist2s commented Jul 5, 2018

Yes, I use Promises in my code. But I can imagine how difficult and costly is to make realization of Promise in Espruino(utilization and other). I can deal without Promises of course, but It will be more verbose code. I suppose, if promises can't be realized natively it should be removed and if some one will need it Promises can be written on JS directly(can't imagine how costly on controller it will be). Promises is really important thing in JS, for example to make async/await functions, which transpiled by Babel really useful when you doing any asynchronous code.

@gfwilliams
Copy link
Member Author

If you use a new Espruino build from travis, those issues that you found should now be fixed. They'll be sorted in 1v100 anyway

@radist2s
Copy link

radist2s commented Jul 6, 2018

Thank you, I tested, order is correct, works great!
But I found another bugs. Don't know is it important, but I will make report.

function foo(bar) {
  console.log(bar);
}

function onInit() {
  const promise = new Promise(function(resolve, reject) {
    // setTimeout(() => reject('OK'), 100);
    // # Normal behaviour

    // setTimeout(() => resolve('OK'), 100);
    // # Normal behaviour
  
    // setTimeout(reject.bind(undefined, 'OK'), 100);
    // # Never executes 'promiseRejected' and event 'promiseCatch'
    // # Exception gets: 'Uncaught Error: Unhandled promise rejection: OK'
    
    // setTimeout(resolve.bind(undefined, 'OK'), 100);
    // # Never resolves
    // # No errors
  });

  promise.then(
    function promiseSucceeded(result) {
      console.log(`Resolved: ${result}`);
    },
    function promiseRejected(error) {
      console.log(`Error: ${error}`);
    }
  );
    
  promise.catch(function promiseCatch(catched) {
    console.log(`Catched: ${catched}`);
  });
}

save();

Just uncomment one of resolve or rejection lines. So, bind is not works correct on Promise constructor arguments.

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