Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: emit after event with proper error param for node versions >= 11.4.0 #1732

Merged
merged 2 commits into from Jan 3, 2019

Conversation

misterdjules
Copy link
Contributor

Pre-Submission Checklist

  • Opened an issue discussing these changes before opening the PR
  • Ran the linter and tests via make prepush
  • Included comprehensive and convincing tests for changes

Issues

Closes:

Summarize the issues that discussed these changes

Restify currently relies on specific timing of the response's 'close' event to emit the 'after' event with the correct error parameter in case the request is aborted by the client.

Changes

What does this PR do?

This PR makes Restify not overwrite any error previously set as res.err so that it is passed as the first parameter when emitting the 'after' event once the req/res lifecycle is finished.

@misterdjules misterdjules requested a review from a team January 1, 2019 00:22
@misterdjules
Copy link
Contributor Author

Note that this is potentially a breaking change, as there are potentially one or more situations where an 'after' event would have been emitted without an error that would now result in an 'after' event being emitted with an error.

I'm not sure this is worth the overhead though, so for now I'm inclined to treat this as a bug fix and keep this as a patch version bump.

@misterdjules misterdjules merged commit 7a1378b into master Jan 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants