restify / restify/node-restify
Error during connection upgrade leads to uncaught write-after-end error
Nobody has claimed this yet.
- Dominant language
- JavaScript
- Stars
- 10.7k
- Forks
- 975
- Avg merge
- 1d 6h
- Merged PRs (30d)
- 5
Description
- 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
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.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start in server.js at the _emitErrorEvents() callback and compare its res._sent guard with upgrade.js, especially _writeHeadImpl() and the emitError() path. Reproduce the connection-upgrade error and verify that websocket-route error handling no longer produces an uncaught write-after-end error.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- javascript, node.js
- Domain
- api, backend
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Stale
- Clarity
- Clearly specified
- Newbie friendliness
- 45/100