Skip to content

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 misterdjules force-pushed the fix-after-event-error-param branch from f84a97a to cc54a93 Compare January 1, 2019 00:24
@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.

3 participants