node-restify icon indicating copy to clipboard operation
node-restify copied to clipboard

Route handler still runs after returning response for earlier uncaught exception

Open hashtagchris opened this issue 7 months ago • 1 comments

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

Restify Version: 11.1.0 (also 10.0.0, 9.1.0, 8.6.1, 7.7.0, but not earlier versions) Node.js Version: v20.15.1

Expected behaviour

I would expect the handler registered for a given route to not execute if an (error) response has already been sent to the client.

I understand middleware handlers possibly running regardless of a response being sent, but I would expect "normal", route-specific handlers to be skipped if there's already been an uncaught exception, and the response was completed.

Actual behaviour

The handler still runs. Unless the handler defensively checks res.headersSent, there's unnecessary processing and an ERR_HTTP_HEADERS_SENT error when the handler attempts to send a response.

Repro case

const restify = require('restify')
const axios = require('axios')

const server = restify.createServer({handleUncaughtExceptions: true})
server.use(restify.plugins.bodyParser())

server.on('uncaughtException', function (req, res, route, err) {
  console.log('restify uncaughtException:', err.code, err.message)
  if (!res.headersSent) {
    res.send(500, {error: err.message})

    // Q. How can I end request processing here, and prevent the registered hello handler from running?
  }
})

server.get('/hello', function (req, res, next) {
  console.log('hello handler called')

  // // I'd prefer not to add defensive code like to this to individual route handlers
  // if (res.headersSent) {
  //   console.log('weird, response already sent, skipping execution')
  //   return
  // }

  res.send(200, {hello: 'world'})
})

server.listen(9595, function () {
  console.log(`${server.name} listening at ${server.url}`)
})

// intentionally send a request with an invalid (empty) gzip body
axios({
  method: 'get',
  url: 'http://localhost:9595/hello',
  headers: {'Content-encoding': 'gzip', 'Content-type': 'application/json'},
  validateStatus: () => true
}).then(response => {
  console.log('server response', response.status, response.data)

  server.close()
})

Output

restify listening at http://[::]:9595
restify uncaughtException: Z_BUF_ERROR unexpected end of file
hello handler called
restify uncaughtException: ERR_HTTP_HEADERS_SENT Cannot set headers after they are sent to the client
server response 500 { error: 'unexpected end of file' }

Cause

Seems to be related to the uncaught exception occurring for an early middleware handler (readBody). This doesn't prevent later handlers from executing.

Are you willing and able to fix this?

No

hashtagchris avatar Jul 10 '24 17:07 hashtagchris