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

Error during connection upgrade leads to uncaught write-after-end error

Open jmattsson opened this issue 5 years ago • 0 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: 8.5.1 Node.js Version: v10.15.2

Expected behaviour

Error handling on web socket routes should not lead to uncaught errors.

Actual behaviour

An error during the connection upgrade results in a write-after-end error.

Workaround

Suppressing the thrown error is possible, if inelegant:

server.on('restifyError', (req, res, err, done) => {
  try { done(); } catch(e) { /* suppress the error */ }
})

Cause

The cause appears to be that for regular (non-upgrade) requests, the_emitErrorEvents() callback in server.js has a guard against double handling by checking res._sent. For upgrade responses, this flag does not exist. Rather, it appears to use res._headWritten. Modifying _writeHeadImpl() in upgrade.js to also set this._sent in addition to this._headWritten avoids the write-after-end error. Alternatively, changing the emitError() function to also check res._headWritten should have the same effect.

Are you willing and able to fix this?

See above for proposed fix. I'm not familiar enough with the project internals to want to PR myself.

jmattsson avatar Aug 26 '20 08:08 jmattsson