Default error handler, next(), and ESLint's no unused var rule

[kevinSuttle]

This may be an Express-specific issue, but I'm trying to wrap my head around it.

Given the below code, ESLint complains that next is never used.

// development error handler
// will print stacktrace
if (app.get('env') === 'development') {
  app.use(function(err, req, res, next) {
    res.status(err.status || 500);
    res.render('error', {
      message: err.message,
      error: err
    });
  });
}

// production error handler
// no stacktraces leaked to user
app.use(function(err, req, res, next) {
  res.status(err.status || 500);
  res.render('error', {
    message: err.message,
    error: {}
  });
});

My questions:

• Isn't next() implicitly called in Node middleware? I thought the best practice was to not call it explicitly in middleware.
• If I do call it, it results in a crash by default. Using /foo as a test route.
  Calling next() by itself

// development error handler—will print stacktrace
if (app.get('env') === 'development') {
  app.use(function(err, req, res, next) {
    res.status(err.status || 500);
    res.render('error', {
      message: err.message,
      error: err
    });
   next();
  });
}

console

GET /foo 404 28.361 ms - 16
_http_outgoing.js:335
    throw new Error('Can\'t set headers after they are sent.');
          ^
Error: Can't set headers after they are sent.
    at ServerResponse.OutgoingMessage.setHeader (_http_outgoing.js:335:11)
    at ServerResponse.header (/Users/kevinsuttle/Code/REDACTED/node_modules/express/lib/response.js:700:10)
    at ServerResponse.send (/Users/kevinsuttle/Code/REDACTED/node_modules/express/lib/response.js:154:12)
    at Stub.fn [as callback] (/Users/kevinsuttle/Code/REDACTED/node_modules/express/lib/response.js:934:10)
    at Stub.flush (/Users/kevinsuttle/Code/REDACTED/node_modules/dustjs-linkedin/lib/dust.js:567:10)
    at Chunk.end (/Users/kevinsuttle/Code/REDACTED/node_modules/dustjs-linkedin/lib/dust.js:704:15)
    at done (/Users/kevinsuttle/Code/REDACTED/node_modules/dustjs-linkedin/lib/dust.js:177:75)
    at /Users/kevinsuttle/Code/REDACTED/node_modules/consolidate/lib/consolidate.js:100:5
    at fs.js:334:14
    at FSReqWrap.oncomplete (fs.js:95:15)
18 May 12:52:23 - [nodemon] app crashed - waiting for file changes before starting...

browser

Cannot GET /foo

Calling next(err)

// development error handler—will print stacktrace
if (app.get('env') === 'development') {
  app.use(function(err, req, res, next) {
    res.status(err.status || 500);
    res.render('error', {
      message: err.message,
      error: err
    });
    next(err)
  });
}

browser

Error: Not Found
   at /Users/kevinsuttle/Code/REDACTED/app.js:56:12
   at Layer.handle [as handle_request] (/Users/kevinsuttle/Code/REDACTED/node_modules/express/lib/router/layer.js:82:5)
   at trim_prefix (/Users/kevinsuttle/Code/REDACTED/node_modules/express/lib/router/index.js:302:13)
   at /Users/kevinsuttle/Code/REDACTED/node_modules/express/lib/router/index.js:270:7
   at Function.proto.process_params (/Users/kevinsuttle/Code/REDACTED/node_modules/express/lib/router/index.js:321:12)
   at next (/Users/kevinsuttle/Code/REDACTED/node_modules/express/lib/router/index.js:261:10)
   at csrf (/Users/kevinsuttle/Code/REDACTED/middleware/utilities.js:5:2)
   at Layer.handle [as handle_request] (/Users/kevinsuttle/Code/REDACTED/node_modules/express/lib/router/layer.js:82:5)
   at trim_prefix (/Users/kevinsuttle/Code/REDACTED/node_modules/express/lib/router/index.js:302:13)
   at /Users/kevinsuttle/Code/REDACTED/node_modules/express/lib/router/index.js:270:7

console

Error: Not Found
    at /Users/kevinsuttle/Code/REDACTED/app.js:56:12
    at Layer.handle [as handle_request] (/Users/kevinsuttle/Code/REDACTED/node_modules/express/lib/router/layer.js:82:5)
    at trim_prefix (/Users/kevinsuttle/Code/REDACTED/node_modules/express/lib/router/index.js:302:13)
    at /Users/kevinsuttle/Code/REDACTED/node_modules/express/lib/router/index.js:270:7
    at Function.proto.process_params (/Users/kevinsuttle/Code/REDACTED/node_modules/express/lib/router/index.js:321:12)
    at next (/Users/kevinsuttle/Code/REDACTED/node_modules/express/lib/router/index.js:261:10)
    at csrf (/Users/kevinsuttle/Code/REDACTED/middleware/utilities.js:5:2)
    at Layer.handle [as handle_request] (/Users/kevinsuttle/Code/REDACTED/node_modules/express/lib/router/layer.js:82:5)
    at trim_prefix (/Users/kevinsuttle/Code/REDACTED/node_modules/express/lib/router/index.js:302:13)
    at /Users/kevinsuttle/Code/REDACTED/node_modules/express/lib/router/index.js:270:7
GET /404 404 21.206 ms - 1150
_http_outgoing.js:335
    throw new Error('Can\'t set headers after they are sent.');
          ^
Error: Can't set headers after they are sent.
    at ServerResponse.OutgoingMessage.setHeader (_http_outgoing.js:335:11)
    at ServerResponse.header (/Users/kevinsuttle/Code/REDACTED/node_modules/express/lib/response.js:700:10)
    at ServerResponse.send (/Users/kevinsuttle/Code/REDACTED/node_modules/express/lib/response.js:154:12)
    at Stub.fn [as callback] (/Users/kevinsuttle/Code/REDACTED/node_modules/express/lib/response.js:934:10)
    at Stub.flush (/Users/kevinsuttle/Code/REDACTED/node_modules/dustjs-linkedin/lib/dust.js:567:10)
    at Chunk.end (/Users/kevinsuttle/Code/REDACTED/node_modules/dustjs-linkedin/lib/dust.js:704:15)
    at done (/Users/kevinsuttle/Code/REDACTED/node_modules/dustjs-linkedin/lib/dust.js:177:75)
    at /Users/kevinsuttle/Code/REDACTED/node_modules/consolidate/lib/consolidate.js:100:5
    at fs.js:334:14
    at FSReqWrap.oncomplete (fs.js:95:15)
18 May 12:58:19 - [nodemon] app crashed - waiting for file changes before starting...

• Should this be a Node-specific rule in ESLint? (I will ask over in their repo, too).

Forgive my newbness

[dougwilson]

Yes, the biggest issue is that Express error middleware is generally not compatible with that rule. You can probably add some kind of ESLint comment to disable that rule just for the error handling function.

The reason it's there, but unused, is because in Express, an error middlware is differentiated from a normal middleware by the function taking 4 arguments rather than 3 arguments (i.e. function (req, res, next) vs function (err, req, res, next)).

You are also getting an error when you manually call next() because in Express, you can only call next() if you have not responded. The res.render statement responds to the client, to the call to next() will cause an error, since you cannot do both.

It boils down to that rule not being compatible with Express error middlewares is all. I have never user ESLint (only JSHint), so I don't know how to tell you what you need to change in your ESLint rules.

[dougwilson]

It looks like here is the issue on ESLint: eslint/eslint#1494 seems they say just don't use that rule.

[kevinSuttle]

Thanks so much, @dougwilson. I have relaxed it to a warning for now.

[zckrs]

Since 2015 the rule no-unused-vars have news options.

You can use http://eslint.org/docs/rules/no-unused-vars#argsignorepattern

Example in package.json

"eslintConfig": {
    "rules": {
      "no-unused-vars": ["error", { "argsIgnorePattern": "next" }]
    }
  }

[rafis]

Is it possible to use no-unused-vars in JSHint?

This is nearest answer I have found.

[gghez]

@zckrs it is a little bit unsafe, custom error handler appears once in your code, so you may prefer using // eslint-disable-line to disable eslint locally.

[vvo]

You can also use:

// eslint-disable-next-line no-unused-vars

To be extra specific

[nvzard]

None of the above-mentioned solutions are working for something like:

    this.express.use((err: Error, req: Request, res: Response, next: NextFunction) => {
      handleError(err, res);
    });

error 'next' is defined but never used @typescript-eslint/no-unused-vars

Had to use:

    /* eslint-disable */
    this.express.use((err: Error, req: Request, res: Response, next: NextFunction) => {
      handleError(err, res);
    });
    /* eslint-enable */

Weird thing is, I am not getting error for unused req but only for next argument.

[adrianosouzacostacr]

My Function

requestHandler(_req: Request, res: Response, _next: Next): Promise<void> {
    return new UserController().list(res);
  }

You can use:

"rules": {
    "@typescript-eslint/no-unused-vars": ["warn", { "argsIgnorePattern": "^_" }]
  }

Result is zero warnings on lint

This pattern is shown in https://eslint.org/docs/rules/no-unused-vars