Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Authenticate callback not being called on err #15

Closed
kslesinsky opened this Issue Aug 31, 2012 · 7 comments

Comments

Projects
None yet
5 participants
Owner

jaredhanson commented Aug 31, 2012

Left a comment on Stack Overflow. It's not a bug, and its working as intended.

That said, there may be compelling reasons to pass errors to a custom callback, and I'm open to that idea. Let me know if you have a use case that isn't covered by error handling middleware.

Owner

jaredhanson commented Aug 31, 2012

Following up here with my recommendation, so I can format code better than at SO.

app.get('/logintest',
  function(req, res, next) {
    console.log('before authenticate');
    passport.authenticate('local', function(err, user, info) {
      console.log('authenticate callback');
      if (err) { return res.send({'status':'err','message':err.message}); }
      if (!user) { return res.send({'status':'fail','message':info.message}); }
      req.logIn(user, function(err) {
        if (err) { return res.send({'status':'err','message':err.message}); }
        return res.send({'status':'ok'});
      });
    })(req, res, next);
  },
  function(err, req, res, next) {
    // failure in login test route
    return res.send({'status':'err','message':err.message});
  });

Note that there is an error handler mounted at the end of this route. Since Passport currently next()'s with an error (rather than supplying it to the callback), this error handler will catch it and you can reply as needed. It's still recommended to check err in the callback, even though as implemented it won't ever be set.

Thanks... I learned something new about nodejs - how to mount an error handler. Much appreciated!

jergason commented Sep 7, 2012

I agree with @kslesinsky that this is very unexpected behavior. Why have an error parameter in the callback to authenticate if it won't actually be called when there is an error? I think it would be much more intuitive if errors passed to the verify callback returned in the authenticate callback.

I think this information is very very useful and should be added to the guide too in order to prevent some confusing situations.

I agree with the above, spent a good amount of time trying to figure out why my callback was not being called when trying to handle an auth error. Eventually found the info here, but if the same example was just added to http://passportjs.org/guide/authenticate/ I'm sure other people would find it beneficial.

Owner

jaredhanson commented Jan 15, 2014

Errors are now passed to custom callbacks. See issue #186 for more information.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment