luin / express-promise

❤️ Middleware for easy rendering of async Query results.
MIT License
316 stars 19 forks source link

HTTP status codes? #8

Closed ghost closed 10 years ago

ghost commented 10 years ago

The example in the readme always assumes a 200 HTTP response (or at least it appears that way):

app.get('/users/:userId', function(req, res) {
  res.json({
    user: User.find(req.params.userId),
    memo: Project.getMemo(req.params.userId)
  });
});

If User.find returns nothing, I'd want to set a 400 error (and this should happen at the request level, not at the model level). Without some way to configure promise reject handling, the usefulness of this library seems negated.

Normally, I'd have to do something like:

app.get('/users/:userId', function(req, res) {
    User
      .find(req.params.userId)
      .then(function(user) {
        res.json(200, {
          status: 'success',
          data: user 
        });
      }, function(error) {
        res.json(400, {
          status: 'fail',
          message: 'User not found.'
        });
      });
});

Any thoughts on this? I took a peak through the code but nothing stuck out.

luin commented 10 years ago

When one of your promises fails, as a typical Express middleware, express-promise will pass the error to the next function(see https://github.com/luin/express-promise/blob/master/lib/express_promise.js#L117). Just print a log to see whether the lib has caught the error successfully.

luin commented 10 years ago

Closing this issue. Feel free to reopen it if you still have any problems.

camjackson commented 10 years ago

I'm having trouble getting this to work. If I initialise mongoose with the wrong database name, then attempting to render a template fails, but the error is never passed to my error handling middleware. I put breakpoints at all the places in the linked file above where next is called, and the only one that got hit was this one. So I end up just getting a 500 back, without my error handling middleware being called.

Probably hard to give much advice without seeing code the reproduces it, but do you have any suggestions?

camjackson commented 10 years ago

Been reading the code a bit more... It looks like if Mongoose returns an error, then I should end up here, but again: I put a breakpoint there and that line never gets hit...

luin commented 10 years ago

MongoDB will automatically create a database if the specific database isn't exist. So typically it won't produce any errors when initialising mongoose with a wrong database name.

Could you figure out what error exactly cause the rending fails?

camjackson commented 10 years ago

Ahh, my mistake. The error wasn't coming directly from Mongoose, which is why express-promise wasn't catching one. It was happening when trying to render the template with missing data (because Mongoose didn't find the data in the empty database that it created when connecting). What I need to do is pass in a callback to res.render so that I can catch errors from the rendering process. Thanks anyway.