unexpected error code for 'ResourceNotFoundError'

[seoker]

Bug Report

Restify Version

5.2.0

Node.js Version

7.10.1

Expected behaviour

// trigger this error when some in-existing route being called
server.on('NotFound', (req, res, err, cb) => {
  if (err instanceof errors.ResourceNotFoundError) {
    // should get here
  }

  cb();
});

Actual behaviour

// trigger this error when some in-existing route being called
server.on('NotFound', (req, res, err, cb) => {
  req.log.info(JSON.stringify(err, null, 2)); // { "code": "ResourceNotFound", "message": "/xxxx does not exist" }
  req.log.info(err.toString()); // ResourceNotFoundError: /xxxx does not exist
  req.log.info(err.code); // Error
  if (err instanceof errors.ResourceNotFoundError) {
    // never get here
  }

  cb();
});

Repro case

1. create a error handler for NotFound error (though I assume there should have a ResourceNotFoundError)
2. call API server with an in-existing route.

[retrohacker]

Hey @seoker,

Thank you for taking the time to fill out our template and for providing example cases.

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

var app = restify.createServer();

// trigger this error when some in-existing route being called
app.on('NotFound', (req, res, err, cb) => {
  console.log(JSON.stringify(err, null, 2)); // { "code": "ResourceNotFound", "message": "/xxxx does not exist" }
  console.log(err.toString()); // ResourceNotFoundError: /xxxx does not exist
  console.log(err.code); // Error
  if (err instanceof errors.ResourceNotFoundError) {
    console.log('CHUMBAWAMBA');
  }

  cb();
});

app.listen(8080);

This outputs

{
  "code": "ResourceNotFound",
  "message": "/ does not exist"
}
ResourceNotFoundError: / does not exist
Error
CHUMBAWAMBA

It is possible that the version of restify-errors that restify is using is different than the one you are using, so the ResourceNotFound objects are actually two different objects. You can check this using npm list and looking to see if you have multiple versions of the module floating around.

[seoker]

I had bumped the restify to 6.0.0 yesterday and the issue no longer exist. When I rollback to restify 5.2.0, the issue appeared again and then I check npm list, the version did not match exactly as you said and that's why the instanceof doesn't work.

$ npm list | grep "restify"                                                                                                                 on git:develop|✚5
├─┬ [email protected]
│ ├─┬ [email protected]
├─┬ [email protected]
├─┬ [email protected]
├─┬ [email protected]

Even though, I think error.code should return ResourceNotFound for me to compare, otherwise there's no way to compare them unless using the trick JSON.parse(JSON.stringify(error)), what you think?

[DonutEspresso]

I think part of the confusion here is that the router returns a ResourceNotFoundError, which the server maps into a NotFound event (vs emitting a ResourceNotFound). This may be worth revisiting in the goal of being consistent. This has been the existing behavior for some time - may have to ping some legacy folks to understand why we do the translation of router errors at the server.

[cjroebuck]

Is the default behaviour of this i.e. / does not exist a bit of a security vulnerability? An attacker could inject text into the error message/404 page easily by linking to some non-existant route like /this,site,is,no,longer,available,please,go,to,attacker.com,existingsite.com and that will return a message like:

{ "message": "/this,site,is,no,longer,available,please,go,to,attacker.com,existingsite.com does not exist", ...}