restify / node-restify

The future of Node.js REST development
http://restify.com
MIT License
10.71k stars 982 forks source link

Restify does not handle Promise rejection #1853

Open expelliamus opened 4 years ago

expelliamus commented 4 years ago

I'm trying to handle promise rejection creating a simple middleware, so I did:

const errorHandler = (err: Error, req: Request, res: Response, next: Next) => {
    console.log('hello world');
}

so:

restify.use(errorHandler);

but surprising I get:

Argument of type '(err: Error, req: Request, res: Response, next: any) => void' is not assignable to parameter of type 'RequestHandlerType'. Type '(err: Error, req: Request, res: Response, next: any) => void' is not assignable to type 'RequestHandler'.ts(2345)

so Restify does not include Error, and for manage the Promise rejection I should wrap this around each route:

export const errorHandler = (callback: Function) => {
    return async function errorHandler(req: Request, res: Response, next: Next) {
        try {
            await callback(req, res, next)
        } catch (err) {
            next(err);
        }
    }
}
server.get('/access-codes/plan/:planId', errorHandler(list));

this could become a pain, why Restify doesn't support Error in RequestHandler?

reediculous456 commented 4 years ago

Restify handles errors differently that express and doesn't use a middleware. (See the docs for this here)

Here is how I handle my errors:

ErrorHandler.js

class ErrorHandler {
  constructor(server) {
    server.on(`restifyError`, this.defaultErrorHandler);
  }

  defaultErrorHandler(req, res, err, callback) {
    console.error(err);
    return callback();
  }
}

index.js

const { createServer } = require(`restify`);
const { ErrorHandler } = require(`./ErrorHandler.js`);

const server = createServer();
new ErrorHandler(server);
expelliamus commented 4 years ago

@reediculous456 I understand but is not what I asked for... suppose you have this:

server.post('/customers', customerService.create);

then you have this:

export const create = async (req: Request, res: Response): Promise<Customer> => {
    throw new Error('Promise rejection error');

none of the following will handle the promise rejection:

server.on(`restifyError`, this.defaultErrorHandler);
server.on(`InternalServer`, this.defaultErrorHandler);

so Restify doesn't provide any way to handle Promise Rejection and this is bad

mmarchini commented 4 years ago

Restify 8 does not support promises as route handlers. Restify 9 will, support is already integrated on master: https://github.com/restify/node-restify/pull/1833

gitblit commented 3 years ago

@mmarchini do you know of an ETA for v9?

nikeee commented 3 years ago

My application heavily depends on that. Any schedule/ETA?

If not, is the current master branch stable? If so, I'd reference this in my package.json.