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
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
feat(req): add restifyDone event
  • Loading branch information
Peter Marton committed Jan 18, 2019
commit e3cc097f9b86ac87c88c8a13829f6e0708b972a0
136 changes: 136 additions & 0 deletions docs/_api/request.md
Original file line number Diff line number Diff line change
@@ -33,6 +33,7 @@ permalink: /docs/request-api/
- [endHandlerTimer](#endhandlertimer)
- [connectionState](#connectionstate)
- [getRoute](#getroute)
- [Events](#events)
- [Log](#log)

## Request
@@ -378,6 +379,141 @@ _Route info object structure:_

Returns **[Object](https://developer.mozilla.org/docs/Web/JavaScript/Reference/Global_Objects/Object)** route

## Events

In additional to emitting all the events from node's
[http.Server](http://nodejs.org/docs/latest/api/http.html#http_class_http_server),
restify servers also emit a number of additional events that make building REST
and web applications much easier.

### restifyError

This event is emitted following all error events as a generic catch all. It is
recommended to use specific error events to handle specific errors, but this
event can be useful for metrics or logging. If you use this in conjunction with
other error events, the most specific event will be fired first, followed by
this one:

```js
server.get('/', function(req, res, next) {
return next(new InternalServerError('boom'));
});

server.on('InternalServer', function(req, res, err, callback) {
// this will get fired first, as it's the most relevant listener
return callback();
});

server.on('restifyError', function(req, res, err, callback) {
// this is fired second.
return callback();
});
```

### after

After each request has been fully serviced, an `after` event is fired. This
event can be hooked into to handle audit logs and other metrics. Note that
flushing a response does not necessarily correspond with an `after` event.
restify considers a request to be fully serviced when either:

1) The handler chain for a route has been fully completed
2) An error was returned to `next()`, and the corresponding error events have
been fired for that error type

The signature is for the after event is as follows:

```js
function(req, res, route, error) { }
```

- `req` - the request object
- `res` - the response object
- `route` - the route object that serviced the request
- `error` - the error passed to `next()`, if applicable

Note that when the server automatically responds with a
NotFound/MethodNotAllowed/VersionNotAllowed, this event will still be fired.

### pre

Before each request has been routed, a `pre` event is fired. This event can be
hooked into handle audit logs and other metrics. Since this event fires
_before_ routing has occured, it will fire regardless of whether the route is
supported or not, e.g. requests that result in a `404`.

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

```js
function(req, res) {}
```

- `req` - the request object
- `res` - the response object

Note that when the server automatically responds with a
NotFound/MethodNotAllowed/VersionNotAllowed, this event will still be fired.

### routed

A `routed` event is fired after a request has been routed by the router, but
before handlers specific to that route has run.

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

```js
function(req, res, route) {}
```

- `req` - the request object
- `res` - the response object
- `route` - the route object that serviced the request

Note that this event will _not_ fire if a requests comes in that are not
routable, i.e. one that would result in a `404`.

### uncaughtException

If the restify server was created with `handleUncaughtExceptions: true`,
restify will leverage [domains](https://nodejs.org/api/domain.html) to handle
thrown errors in the handler chain. Thrown errors are a result of an explicit
`throw` statement, or as a result of programmer errors like a typo or a null
ref. These thrown errors are caught by the domain, and will be emitted via this
event. For example:

```js
server.get('/', function(req, res, next) {
res.send(x); // this will cause a ReferenceError
return next();
});

server.on('uncaughtException', function(req, res, route, err) {
// this event will be fired, with the error object from above:
// ReferenceError: x is not defined
});
```

If you listen to this event, you **must** send a response to the client. This
behavior is different from the standard error events. If you do not listen to
this event, restify's default behavior is to call `res.send()` with the error
that was thrown.

The signature is for the after event is as follows:

```js
function(req, res, route, error) { }
```

- `req` - the request object
- `res` - the response object
- `route` - the route object that serviced the request
- `error` - the error passed to `next()`, if applicable

### close

Emitted when the server closes.


## Log

If you are using the [RequestLogger](#bundled-plugins) plugin, the child logger
21 changes: 21 additions & 0 deletions docs/api/request-events.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
### restifyDone

After request has been fully serviced, an `restifyDone` event is fired.
restify considers a request to be fully serviced when either:

1) The handler chain for a route has been fully completed
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/''


```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...

```

* `route` - the route object that serviced the request
* `error` - the error passed to `next()`, if applicable

Note that when the server automatically responds with a
`NotFound`/`MethodNotAllowed`/`VersionNotAllowed`, this event will still be
fired.
2 changes: 2 additions & 0 deletions docs/config/request.yaml
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
toc:
- Request
- name: Events
file: ../api/server-events.md
- name: Log
file: ../api/request-log.md
4 changes: 3 additions & 1 deletion lib/server.js
Original file line number Diff line number Diff line change
@@ -1256,7 +1256,9 @@ Server.prototype._finishReqResCycle = function _finishReqResCycle(
req._timeFinished = process.hrtime();

// after event has signature of function(req, res, route, err) {...}
self.emit('after', req, res, route, err || res.err);
var finalErr = err || res.err;
req.emit('restifyDone', route, finalErr);
self.emit('after', req, res, route, finalErr);
} else {
// Store error for when the response is flushed and we actually emit the
// 'after' event. The "err" object passed to this method takes
21 changes: 21 additions & 0 deletions test/request.test.js
Original file line number Diff line number Diff line change
@@ -223,3 +223,24 @@ test('should provide date when request started', function(t) {
t.end();
});
});

// 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?

var clientDone = false;

SERVER.get('/', function(req, res, next) {
res.send('hello');
req.on('restifyDone', function() {
t.ok(clientDone);
t.end();
});
return next();
});

CLIENT.get('/', function(err, _, res) {
t.ifError(err);
t.equal(res.statusCode, 200);
clientDone = true;
});
});