Errors event chain incoherent, 'restifyError' not always emitted

[simonepri]

• Used appropriate template for the issue type
• Searched both open and closed issues for duplicates of this issue
• Title adequately and concisely reflects the feature or the bug

Bug Report

restifyError event is not emitted for errors generated internaly by Restify.
(NotFound, MethodNotAllowed, VersionNotAllowed, UnsupportedMediaType)

Restify Version

restify@5.0.1

Node.js Version

node@8.1.4

Expected behaviour

restifyError must be called also for errors generated internally by Restify!

Actual behaviour

Hit ->http://localhost:2300/notfound
Cause this:

Hit -> http://localhost:2300/test
Cause this:

As per my opinion is really incoherent.

Repro case

const restify = require('restify');
const errors = require('restify-errors');

server = restify.createServer();

server.get('/test', (req, res, next) => {
  return next(new errors.NotFoundError());
});

server.listen(2300, () => {
  console.log('%s listening at %s', server.name, server.url);
});

server.on('NotFound', (req, res, err, cb) => printEvent('NotFound', cb));
server.on('MethodNotAllowed', (req, res, err, cb) => printEvent('MethodNotAllowed', cb));
server.on('VersionNotAllowed', (req, res, err, cb) => printEvent('VersionNotAllowed', cb));
server.on('UnsupportedMediaType', (req, res, err, cb) => printEvent('UnsupportedMediaType', cb));
server.on('restifyError', (req, res, err, cb) => printEvent('restifyError', cb));

server.on('after', () => printEvent('after', () => {}));

function printEvent(event, cb) {
  console.log(event);
  cb();
}

Workaround

const restify = require('restify');
const errors = require('restify-errors');

server = restify.createServer();

server.get('/test', (req, res, next) => {
  return next(new errors.NotFoundError());
});

server.listen(2300, () => {
  console.log('%s listening at %s', server.name, server.url);
});

server.on('NotFound', fixChain);
server.on('MethodNotAllowed', fixChain);
server.on('VersionNotAllowed', fixChain);
server.on('UnsupportedMediaType', fixChain);
server.on('restifyError', restifyErrorHandler);

function fixChain(req, res, err, cb) {
  server.emit('restifyError', req, res, err, cb);
}

function restifyErrorHandler(req, res, err, cb) {
  if (res.fix)  {
    cb();
    return;
  }
  res.fix = true;

  // Your code here
  console.log('restifyError');
  cb();
}

Cause

The problem is in emitRouteError
https://github.com/restify/node-restify/blob/5.x/lib/server.js#L199

Possible solution

if (server.listeners(name).length > 0) {
  hasErrListeners = true;
  errEvtNames.push(name);
}
if (server.listeners('restifyError').length > 0) {
  hasErrListeners = true;
  errEvtNames.push('restifyError');
}
if (hasErrListeners) {
  // Sends all errEvtNames maybe in the same way as here:
  // https://github.com/restify/node-restify/blob/5.x/lib/server.js#L939
} else {
  res.send(err);
  server._finishReqResCycle(req, res, null, err);
}

Are you willing and able to fix this?

"Yes"

[simonepri]

The Documentation isn't clear about that:

Here it says that only errors passed to next() cause restifyError to be emitted (Current misleading behaviour)

While here:

It says that catch errors of all types. (The expected behaviour)

[simonepri]

Closing in favor of:
#1419
#1420