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

res.redirect happening before req.flash #23

Open
franckl opened this issue Aug 25, 2015 · 9 comments
Open

res.redirect happening before req.flash #23

franckl opened this issue Aug 25, 2015 · 9 comments

Comments

@franckl
Copy link

franckl commented Aug 25, 2015

Hi

I am using express 4 and connect-mongo.

I have the following code in a route:

if (doc.pwdResetExpiration < new Date()) {
    req.flash('error', 'Your recovery token has expired, please try again.');
    return res.redirect('/account-recovery');

it can happen that the redirect is processed before the flash message in stored in the session (so it is never flashed to the user). It happens ~20% of the time

The output of console.log(req.flash()) in /account-recovery when repeatedly loading the above route :

{ error: [ 'Your recovery token has expired, please try again.' ] }  ==> it flashed ok
{} ==> the flash was not stored in time
{ error:  ==> it flashed ok again
   [ 'Your recovery token has expired, please try again.',
     'Your recovery token has expired, please try again.' ] }

Is there an easy way to make sure the flash message is stored before executing the redirect ?

@backflip
Copy link

I'm experiencing the same issue after switching to connect-couchdb for session storage.

Update: Happens on my local machine only, server works fine. Same node versions (0.12.7) and npm packages.

Update 2: Switching to a different session storage mechanism fixed the issue. I have reported the problem in tdebarochez/connect-couchdb#32

@franckl
Copy link
Author

franckl commented Sep 24, 2015

see here expressjs/session#157 & expressjs/session#69

@ericlowry
Copy link

I don't know if this helps, but I wrote my own flash message handler and had the same problem. My handler worked great in my dev environment but didn't work in the real world (when I used a remote server as the session store's back end). The only solution I could get to work was to change the way req.flash() store's the message into the session... you need to add a session.save() after you store the message and then you need to wait for the session to actually store it's state before continuing.

Here is some Pseudo code for how I did it:

req.flash = function( key, value, callback ){

  session.flash[ key ] = value;
  session.save( function(err){  // force the session to save
    if (err) { return callback(err); }
    callback();
  });

};

meanwhile, in your express code...

router.get('/update', function( req,res,next ){
  if ( !req.session.premiumUser ){
    req.flash('info','you need to upgrade your account to do that', function(err){
      if (err) { return next(err); }
      res.redirect('/profile');
    });
  }

  // ....
});

The callback gives flash() the time it needs to store the message into whatever the underlying connect store is, before calling the redirect...

Does that make sense?

@franklioxygen
Copy link

got same issue

@iwaduarte
Copy link

Can I bring this issue to the table again? There is a unexpected behavior using flash and session storage when using databases ( connect-sequelize* middleware and so on). The flash message is only displayed if I refresh the browser.What can I do to solve this thing? Please help me. I have been search for quite awhile and I cannot find way to solve this problem.

@davidmendesweb
Copy link

davidmendesweb commented May 21, 2018

Try using res.render() instead of res.redirect() with the necessary arguments (if possible). This may solve your problem in the short term.

Assuming your using express-session you can save the session and then in the callback function redirect to a route. Documentation is at https://www.npmjs.com/package/express-session#sessionsavecallback

req.session.save(function(err) {
  // session saved
  console.log('session saved');
  res.redirect('/someRoute');
});

@malcolmocean
Copy link

Guys guys guys I figured it out. At least for me.

I was halfway through rolling my own new version of req.flash, when I was looking through the docs of express-session and came across this gem:

Note Since version 1.5.0, the cookie-parser middleware no longer needs to be used for this module to work. This module now directly reads and writes cookies on req/res. Using cookie-parser may result in issues if the secret is not the same between this module and cookie-parser.

And lo, I had these lines:

app.use(require('cookie-parser')())
const session = require('express-session')
const MongoStore = require('connect-mongo')(session)
app.use(session({
  secret: process.env.SESSION_STORE_SECRET,
  store: new MongoStore({mongooseConnection: mongoose.connection}),
  maxAge: 10*365*24*60*60*1000, // set to 10 years
  resave: false,
  saveUninitialized: false
}))

Once I changed the cookie-parser line to:

app.use(require('cookie-parser')(process.env.SESSION_STORE_SECRET))`

it worked exactly as expected!

(For some people, the answer will be to remove cookie-parser altogether.)

@CameronSima
Copy link

Same issue. It occurred when I changed from mongo session store on local machine to typeorm store using a remote postgres server. So like other suggested it seems like some kind of race condition. I feel flash should provide a callback to ensure setting the flash blocks until it's done whatever it's doing before the redirect can happen.

@tanloc0598
Copy link

I have the same problem and I found out it only happend when I use : session-file-store.
After I remove session-file-store, flash worked correctly.

Old code:

var FileStore = require('session-file-store')(session)
app.use(session({
  secret: process.env.SECRET ,
  cookie: {
    maxAge: 1000 * 36000
  },
  saveUninitialized: true,
  resave:false,
  store: new FileStore({
   retries: 0, //Giveup unknowns session without any retry
  })
}));

Changed code:

app.use(session({
  secret: process.env.SECRET ,
  cookie: {
    maxAge: 1000 * 36000
  },
  saveUninitialized: true,
  resave:false,
}));

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

9 participants