Promise.resolve returns Undefined

Question

This is a continuation of Multiple Promises - Where to resolve? where I use the same code for different function.

However, this time the Promise.resolve is returning undefined.

Results Thanks to multiple people pointing out the mistakes. There are multiple errors in the code which I realised I committed.

1) Using && in a non Boolean operation.

should use

(console.log(results) , Promise.resolve(results)

instead of

console.log(results) && Promise.resolve(results)

2) Using unneeded Promise.resolve - just return the results from the Async function will yield the same result as using Promise.resolve.

My final codes.

getMessages: function (roomId) {

    return keysAsync('room:'+roomId)
    .then(room => 
        room === '' ? Promise.reject('Invalid room Id')
                    : smembersAsync('room:messages:'+roomId))
        .then(messagesId => { return messagesId })
        .catch(err => { return err }))

}

Original Question I'm using nodejs promisify so I have the followings declared as promise for Redis

const { promisify } = require('util');
const getAsync = promisify(client.get).bind(client);
const hmsetAsync = promisify(client.hmset).bind(client);
const hsetAsync = promisify(client.hset).bind(client);
const incrAsync = promisify(client.incr).bind(client);
const smembersAsync = promisify(client.smembers).bind(client);
const keysAsync = promisify(client.keys).bind(client);
const sismemberAsync = promisify(client.sismember).bind(client);

getMessages: function (roomId) {

    return keysAsync('room:'+roomId)
    .then(room => 
        room === '' ? Promise.reject('Invalid room Id')
                    : smembersAsync('room:messages:'+roomId))
        .then(messagesId => console.log(messagesId) && Promise.resolve(messagesId))
        .catch(err => Promise.reject(err))

},

And then i call the function as follows

tools.getMessages('4').then((results) => {
    console.log('Messages in Room 4 => ', results);
  }).catch(err => console.log(err))

In my console, I can see the following results

[ '191', '192', '193', '194', '195', '196', '197',
'198', '199', '200', '201', '202', '207', '208', '209', '210', '211', '212', '213', '214', '215', '216', '217', '218' ] //this is when i console log messagesId

Messages in Room 4 => undefined //This is when i console log results

None of those calls to `Promise.resolve` or `Promise.reject` are necessary or useful. — T.J. Crowder, Apr 21 '18 at 16:32
The last `.then()` returns the value of `console.log()` which is undefined. — JJJ, Apr 21 '18 at 16:34
And yet, despite that, it's probably not the problem. We'll need to see (MCVE versions of) `keysAsync` and `smembersAsync`. Please update your question with a [mcve] demonstrating the problem, ideally a **runnable** one using Stack Snippets (the `[<>]` toolbar button; [here's how to do one](https://meta.stackoverflow.com/questions/358992/ive-been-told-to-do-a-runnable-example-with-stack-snippets-how-do-i-do-tha)). — T.J. Crowder, Apr 21 '18 at 16:34
@JJJ: Yeah. I assumed the undefined the OP mentioned was the one it logged, but... — T.J. Crowder, Apr 21 '18 at 16:35
There's 2 console.log, one in the getMessages, one in the call. Also i've updted the codes without Promise.All — Someone Special, Apr 21 '18 at 16:35
`smembersAsync` seems to return a Promise, why not just return the function execution? — Luca Kiebel, Apr 21 '18 at 16:37
@T.J.Crowder Thanks for the response. I'm using nodejs and just promisify the redis functions. — Someone Special, Apr 21 '18 at 16:37
Besides all `console.log(messagesId) && Promise.resolve(messagesId)` won't even run `Promise.resolve(messagesId)` since `console.log()` return value is `undefined` and will cut the short circuit done by `&&`. — Redu, Apr 21 '18 at 16:37
@Redu console.log(messagesId) && Promise.resolve(messagesId) runs and returns all the numbers. But the Promise.resolve returns undefined. — Someone Special, Apr 21 '18 at 16:39
@Luca Brilliant! I tried and that works, but is there a reason why my code now doesn't work? — Someone Special, Apr 21 '18 at 16:39
Make it like `.then(messagesId => (console.log(messagesId), Promise.resolve(messagesId)))`. — Redu, Apr 21 '18 at 16:42
@SomeoneSpecial You really should not use `&&` if you are not doing a boolean operation. Use proper statements (`console.log(…); return …;`) or at least the comma operator (`(console.log(…), …)`). — Bergi, Apr 21 '18 at 17:00

Barmar · Accepted Answer · 2018-04-21T17:07:30.217

3

console.log() returns undefined, which is falsey. && is a short-circuiting operator, and only evaluates the second expression when the first expression is truthy. So it's never executing Promise.resolve(messagesId).

Instead of &&, use the comma operator. It evaluates both of its expressions and returns the second one.

    .then(messagesId => (console.log(messagesId), Promise.resolve(messagesId)))

edited Apr 21 '18 at 17:07

answered Apr 21 '18 at 16:38

Barmar

741,623
53
500
612

2

Or stop trying to cram multiple statements into one and use braces with two statements (the more readable solution IMO). – jfriend00 Apr 21 '18 at 16:44
1

And why even use `Promise.resolve()` at all. That seems unnecessary. – jfriend00 Apr 21 '18 at 16:46
I looked at that line and saw `||`. `||` wasn't there, but it's what I saw. Sigh. – T.J. Crowder Apr 21 '18 at 17:02
1

+1 for suggesting the comma operator, -1 for not using it in your example code. You cannot use the comma operator in a concise arrow body - you are actually calling `then` with two arguments. – Bergi Apr 21 '18 at 17:02
@Bergi Thanks. I even answered a question a day or two ago about the dangers of comma operators, yet I fell into the trap. – Barmar Apr 21 '18 at 17:08
@T.J.Crowder I'm not going to address that. I haven't really examined the rest of the logic. – Barmar Apr 21 '18 at 17:09
No logic to analyze. :-) Returning `Promise.resolve(messageId)` from a `then` handler is completely pointless; it's the same as simply returning `messageId` (but with extra unnecessary plumbing). – T.J. Crowder Apr 21 '18 at 17:25
@jfriend00 you have pointed out Promise.resolve() is unnecessary but it wasn't useful to beginner like me because we don't understand. Now I understand that the redisAsync commands results are already a promise object, and so I can just return it. – Someone Special Apr 22 '18 at 11:36
@SomeoneSpecial - See the answer I provided here for an explanation of why you don't need `Promise.resolve()` in this case. – jfriend00 Apr 22 '18 at 16:53

jfriend00 · Answer 2 · 2018-04-22T16:53:27.453

As has already been explained, you are getting the return value of console.log(messagesId) as your resolved value which is undefined because of the way the && works and what console.log() returns.

But since, all you're really trying to do here is log the result and continue with the same resolved value, I'd suggest a cleaner way of doing that is to make yourself a little utility function:

function log(arg) {
    console.log(arg);
    return arg;
}

Then, you can change this:

.then(messagesId => console.log(messagesId) && Promise.resolve(messagesId))

to this:

.then(log)

Notice that within a .then() handler any plain value you return will become the resolved value of the promise chain. You can also return a promise if you want to add another async operation into the promise chain, but if you already have a value, you can just return value;. You don't need to return Promise.resolve(value);. Inside a .then() handler, the Promise.resolve() is just extra, unnecessary code - just return the value directly.

Luca Kiebel · Answer 3 · 2018-04-21T17:02:39.233

0

The Promise doesn't resolve with messageId, because console.log() returns undefined:

console.log(Boolean(console.log("test")));

But this is not the only thing that you should change, in fact, you can leave out a lot, because the smembersAsync() function returns a promise, that you can simply return:

instead of

return keysAsync('room:'+roomId)
    .then(room => 
        room === '' ? Promise.reject('Invalid room Id')
                    : smembersAsync('room:messages:'+roomId))
        .then(messagesId => console.log(messagesId) && Promise.resolve(messagesId))
        .catch(err => Promise.reject(err))

simply use

return keysAsync('room:'+roomId)
    .then(room => {
        if (room === "") { Promise.reject('Invalid room Id') } 
        else return smembersAsync('room:messages:'+roomId)
    })

edited Apr 21 '18 at 17:02

answered Apr 21 '18 at 16:49

Luca Kiebel

9,790
7
29
44

There's no unresolved promise here. There's a resolved value that is `undefined`. – jfriend00 Apr 21 '18 at 16:55
Where is an unresolved Promise in my code? I either return _a_ promise, or reject one on the given condiction – Luca Kiebel Apr 21 '18 at 16:58
Your first sentence says "The Promise is never resolved". That's incorrect. – jfriend00 Apr 21 '18 at 16:59
So, what happens to that promise instead? – Luca Kiebel Apr 21 '18 at 17:00
It's resolved with a resolved value of `undefined`. That's a completely different thing than not getting resolved at all. – jfriend00 Apr 21 '18 at 17:01
Ok, neat to know, I thought the resolver was never called in the first place – Luca Kiebel Apr 21 '18 at 17:01

Promise.resolve returns Undefined

3 Answers3

Linked