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

Promises should ignore a second resolve/reject #1433

Closed
gfwilliams opened this issue May 21, 2018 · 9 comments
Closed

Promises should ignore a second resolve/reject #1433

gfwilliams opened this issue May 21, 2018 · 9 comments

Comments

@gfwilliams
Copy link
Member

http://forum.espruino.com/conversations/316643/#comment14243054

const p = new Promise(function(resolve, reject) {
  setTimeout(resolve, 100);
  setTimeout(reject, 200);
}).then(function() {
    console.log('resolved');
}).catch(function() {
    console.log('rejected');
});
@MaBecker
Copy link
Contributor

Hmm, just tested with 2v09.134 Linux and go this output when testing the code above.

=Promise: {  }
resolved
Uncaught Error: Unhandled promise rejection: undefined
> 

@MaBecker MaBecker reopened this Oct 16, 2021
@gfwilliams
Copy link
Member Author

Thanks! Not sure what happened there then

@MaBecker
Copy link
Contributor

Any hint how to fix this?

@gfwilliams
Copy link
Member Author

looking at what changed in the PR that supposedly fixed this is probably a good start:

f1d7afd

I guess what's interesting is 'rejected' is now not printed. However the rejection is caught by the OS - so maybe the 'unhandled promise' code doesn't check the state of "done"?

@MaBecker
Copy link
Contributor

Wow, there have been many changes in jswrap_promise.c since f1d7afd.

Looks like is could be the section

if (resolve)
jsvObjectSetChild(promise, JS_PROMISE_RESOLVED_NAME, data);
if (fn) {
_jswrap_promise_resolve_or_reject(promise, data, fn);
jsvUnLock(fn);
} else {
if (!resolve)
jsExceptionHere(JSET_ERROR, "Unhandled promise rejection: %v", data);
}

Just added some prints

  jsiConsolePrintf("L121 resolve: %d\n",resolve);
  if (resolve) 
    jsvObjectSetChild(promise, JS_PROMISE_RESOLVED_NAME, data);
  if (fn) {
    _jswrap_promise_resolve_or_reject(promise, data, fn);
    jsvUnLock(fn);
  } else {
    if (!resolve) {
      jsiConsolePrintf("L129 resolve: %d\n",resolve);
      jsExceptionHere(JSET_ERROR, "Unhandled promise rejection: %v", data);
    }
  }

and that's the output:

>L121 resolve: 1
resolved
L121 resolve: 1
>L121 resolve: 0
L129 resolve: 0
Uncaught Error: Unhandled promise rejection: undefined
> 

Hope this helps you to fix it, because I have no clue.

@MaBecker
Copy link
Contributor

What about making jsExceptionHere(JSET_ERROR, "Unhandled promise rejection: %v", data); dependent on data?

  
  } else {
    if (!resolve && data)
      jsExceptionHere(JSET_ERROR, "L141 Unhandled promise rejection:  %v", data);
  }

@MaBecker
Copy link
Contributor

Result of test with this change:

test_promises.out.txt

@gfwilliams
Copy link
Member Author

I'm not sure that helps? try:

const p = new Promise(function(resolve, reject) {
  setTimeout(resolve, 100);
  setTimeout(reject, 200, "Oh no");
}).then(function() {
    console.log('resolved');
}).catch(function() {
    console.log('rejected');
});

@MaBecker
Copy link
Contributor

Just tested - works perfect - many thanks!

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