session icon indicating copy to clipboard operation
session copied to clipboard

Feature request, return promises

Open Soviut opened this issue 7 years ago • 5 comments
trafficstars

In conjunction with the callbacks, it would be nice if session.save() returned a promise as well. This would make in-lining session operations with if blocks, try/catch blocks and loops a lot more straightforward since async/await could be used.

For example, I'm running into race conditions with redirecting happening too soon. So in situations where I need to set the session, like a "flash" message, I have to do this:

try {
  // some error prone operation
} catch(err) {
  flash('error', 'something went wrong')
}
req.session.save((err) => {
  res.redirect('/things')
})

You'll notice session.save() is called regardless of whether the error occurs. I'm choosing to do that instead of duplicate the res.redirect() code into multiple places.

However, what I'd rather have is:

try {
  // some error prone operation
} catch(err) {
  flash('error', 'something went wrong')
  await req.session.save()
}
res.redirect('/things')

Soviut avatar Aug 28 '18 09:08 Soviut

in the meantime could you do something like:

try { // some error prone operation } catch(err) { flash('error', 'something went wrong') await new Promise((resolve,reject) =>{ req.session.save((err)=>{ resolve(); }) }) } res.redirect('/things')

Slasher4k avatar Nov 01 '18 04:11 Slasher4k

It'd be nice to have promises returned elsewhere where callbacks are used as well, e.g., destroy.

brettz9 avatar Sep 15 '19 14:09 brettz9

There are 2 ways:

  1. Always return a promise, even if a callback is passed in.
  2. Return a promise if no callback was provided.

Many popular libraries follow the second way, e.g. fs-extra or ioredis, so they support both callback and promise APIs. And now as I see this is returned from all session methods. So, perhaps, the second way is better.

@dougwilson

If I create a pull request with this feature, will you accept it? And what way to choose?

RussCoder avatar Oct 10 '19 14:10 RussCoder

i wish promise applied!! :)

sangw0804 avatar Nov 01 '19 05:11 sangw0804

@RussCoder No matter what, the operation is async so a return value of anything other than a promise seems kind of pointless. I've seen it done where returning a promise AND calling the callback are possible if supplied.

Soviut avatar Nov 02 '19 07:11 Soviut