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

feat(req): add restifyDone event #1740

Merged
merged 2 commits into from Jan 19, 2019
Merged

feat(req): add restifyDone event #1740

merged 2 commits into from Jan 19, 2019

Conversation

hekike
Copy link
Member

@hekike hekike commented Jan 18, 2019

Emit a restifyDone event on the req object when the request is "fully served" (see docs in this PR).

2) An error was returned to `next()`, and the corresponding error events have
been fired for that error type

The signature is for the `restifyDone` event is as follows:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/is/''

The signature is for the `restifyDone` event is as follows:

```js
function(route, error) { }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason not to match the API of server after?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just thought it would look strange that we pass req in a req event...


// restifyDone is emitted at the same time when server's after event is emitted,
// you can find more comprehensive testing for `after` lives in server tests.
test('should emit restifyDone event when request is fully served', function(t) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we add a test for next(err) too?

@hekike hekike merged commit 4900d6b into master Jan 19, 2019
@hekike hekike deleted the feat/req-restifyDone branch January 19, 2019 00:19
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