fortune: Exception from hook not handled

I would like to validate my request (get, post, update or delete), and if the request is not valid, I would like to return an error (status 400 or 500 depending on the error). For the moment I’m using hooks for this task like this

const fortune = require('fortune');
const status = require('./model').status;
const { errors: { BadRequestError } } = fortune;

function input(context, record, update) {
  switch (context.request.method) {
    case 'create':
      if (status.indexOf(record.status) === -1) {
        throw new BadRequestError('Invalid status');
      } else {
        return record;
      }
    case 'update':
      if (!update || !update.replace || !update.replace.status) {
        return update;
      }

      if (status.indexOf(update.replace.status) === -1) {
        throw new BadRequestError('Invalid status');
      } else {
        return update;
      }
    default: return null;
  }
}

The important line here is throw new BadRequestError('Invalid status');. This line makes fortune returning a 400 error with invalid status as a content.

Even if I get the desire response, it seems that the error is not fully catch, and so I also get the error in the catch of the listener.

app.use((req, res) => createListener(req, res).catch(err => logger.error(err)));

So here are my questions:

Is it a desire behaviour ?
Is it the correct way to validate an entry in the hook ?
Is it possible to handle exceptions from hooks into fortunejs ?

About this issue

Original URL
State: open
Created 8 years ago
Comments: 17 (11 by maintainers)

Most upvoted comments

I found the problem: the listener returns a promise that doesn’t get a catch. The readme in fortune-http states:

const server = http.createServer((request, response) =>
  listener(request, response)
  // Make sure to catch Promise rejections.
  .catch(error => {
    console.error(error.stack)
  }))

I’m not sure what to do about it. I think it’s somewhat unexpected for there to be a return value from the listener, I guess I could remove it but it would be a breaking change. Or I can make sure that this listener never throws.

Though I would be in favor of removing it and bumping major version, not sure what I was thinking with this, it makes dealing with HTTP-specific requests easier but it’s kind of unexpected.

gr0uch on Dec 16, 2017

One more thing, even if I catch the error on this line

app.use((req, res) => createListener(req, res).catch(err => /*Do something here*/));

I can still see

(node:20623) UnhandledPromiseRejectionWarning: Unhandled promise rejection (rejection id: 1): BadRequestError: Invalid status

in the logs. Also if I replace this line

app.use((req, res) => createListener(req, res).catch(err => logger.error(err)));

by this one

app.use(createListener);

I can see in the logs the error twice.

(node:20694) UnhandledPromiseRejectionWarning: Unhandled promise rejection (rejection id: 1): BadRequestError: Invalid status
(node:20694) UnhandledPromiseRejectionWarning: Unhandled promise rejection (rejection id: 2): BadRequestError: Invalid status

thomasthiebaud on Dec 7, 2016