Skip to content

Commit

Permalink
Fix for 557: Provide request headers in various payload events (#562)
Browse files Browse the repository at this point in the history
Closes #557
  • Loading branch information
bodawei authored and arb committed Aug 23, 2017
1 parent 301307b commit 549662a
Show file tree
Hide file tree
Showing 6 changed files with 117 additions and 7 deletions.
4 changes: 3 additions & 1 deletion API.md
Expand Up @@ -20,7 +20,7 @@ as general events are a process-wide facility and will result in duplicated log

## Options
- `[includes]` - optional configuration object
- `[request]` - array of extra hapi request object fields to supply to reporters on "request" events. Valid values ['headers', 'payload']. Defaults to `[]`.
- `[request]` - array of extra hapi request object fields to supply to reporters on "request", "response", and "error" events. Valid values ['headers', 'payload']. Defaults to `[]`.
- `[response]` - array of extra hapi response object fields to supply to reporters on "response" events. Valid values ['payload']. Defaults to `[]`.
- `[ops]` - options for controlling the ops reporting from good. Set to `false` to disable ops monitoring completely.
- `config` - options passed directly into the [`Oppsy`](https://github.com/hapijs/oppsy) constructor as the `config` value. Defaults to `{}`
Expand Down Expand Up @@ -169,6 +169,7 @@ Event object associated with 'error' events.
- `pid` - the current process id.
- `error` - the raw error object.
- `config` - plugin-specific config object combining `request.route.settings.plugins.good` and `request.plugins.good`. Request-level overrides route-level. Reporters could use `config` for additional filtering logic.
- `headers` - the request headers if `includes.request` includes "headers"

The `toJSON` method of `GreatError` has been overwritten because `Error` objects can not be stringified directly. A stringified `GreatError` will have `error.message` and `error.stack` in place of the raw `Error` object.

Expand Down Expand Up @@ -243,6 +244,7 @@ Event object associated with the "request" event. This is the hapi event emitter
- `method` - method used by the request. Maps to `request.method`.
- `path` - incoming path requested. Maps to `request.path`.
- `config` - plugin-specific config object combining `request.route.settings.plugins.good` and `request.plugins.good`. Request-level overrides route-level. Reporters could use `config` for additional filtering logic.
- `headers` - the request headers if `includes.request` includes "headers"

### `WreckResponse`

Expand Down
2 changes: 1 addition & 1 deletion CONTRIBUTING.md
Expand Up @@ -10,5 +10,5 @@ Code changes are welcome and should follow the guidelines below.
* Fork the repository on GitHub.
* Fix the issue ensuring that your code follows the [style guide](https://github.com/hapijs/contrib/blob/master/Style.md).
* Add tests for your new code ensuring that you have 100% code coverage (we can help you reach 100% but will not merge without it).
* Run `npm run test-cov-html` to generate a report of test coverage
* Run `npm run test` to generate a report of test coverage
* [Pull requests](http://help.github.com/send-pull-requests/) should be made to the [master branch](https://github.com/spumko/good/tree/master).
4 changes: 2 additions & 2 deletions lib/monitor.js
Expand Up @@ -42,7 +42,7 @@ class Monitor {

this._requestLogHandler = (request, event) => {

this.push(() => new Utils.RequestLog(request, event));
this.push(() => new Utils.RequestLog(reqOptions, request, event));
};

this._logHandler = (event) => {
Expand All @@ -52,7 +52,7 @@ class Monitor {

this._errorHandler = (request, error) => {

this.push(() => new Utils.RequestError(request, error));
this.push(() => new Utils.RequestError(reqOptions, request, error));
};

this._responseHandler = (request) => {
Expand Down
12 changes: 10 additions & 2 deletions lib/utils.js
Expand Up @@ -29,7 +29,7 @@ class ServerLog {

// Payload for "error" events
class RequestError {
constructor(request, error) {
constructor(reqOptions, request, error) {

this.event = 'error';
this.timestamp = request.info.received;
Expand All @@ -39,6 +39,10 @@ class RequestError {
this.pid = process.pid;
this.error = error;
this.config = internals.extractConfig(request);

if (reqOptions.headers) {
this.headers = Hoek.reach(request, 'raw.req.headers');
}
}
toJSON() {

Expand Down Expand Up @@ -132,7 +136,7 @@ class Ops {

// Payload for "request" events via request.log
class RequestLog {
constructor(request, event) {
constructor(reqOptions, request, event) {

this.event = 'request';
this.timestamp = event.timestamp;
Expand All @@ -143,6 +147,10 @@ class RequestLog {
this.method = request.method;
this.path = request.path;
this.config = internals.extractConfig(request);

if (reqOptions.headers) {
this.headers = Hoek.reach(request, 'raw.req.headers');
}
}
}

Expand Down
100 changes: 100 additions & 0 deletions test/monitor.js
Expand Up @@ -703,6 +703,55 @@ describe('Monitor', () => {
], done);
});

it('includes headers in the "error" data object if so configured', { plan: 3 }, (done) => {

const server = new Hapi.Server({ debug: false });
server.connection();

server.route({
method: 'GET',
path: '/',
handler: (request, reply) => {

throw new Error('mock error');
}
});

const out = new GoodReporter.Writer(true);
const monitor = internals.monitorFactory(server, {
includes: {
request: ['headers'],
response: []
},
reporters: { foo: [out] }
});

AsyncSeries([
server.start.bind(server),
monitor.start.bind(monitor),
(callback) => {

server.inject({
url: '/',
headers: {
'X-Magic-Value': 'rabbits out of hats'
}
}, (res) => {

expect(res.statusCode).to.equal(500);
setTimeout(() => {

expect(out.data).to.have.length(2);

const event = out.data[1];
expect(event.headers['x-magic-value']).to.equal('rabbits out of hats');
server.stop(callback);
}, 50);
});
}
], done);
});

it('has a standard "log" data object', { plan: 3 }, (done) => {

const server = new Hapi.Server();
Expand Down Expand Up @@ -787,6 +836,57 @@ describe('Monitor', () => {
], done);
});

it('includes headers in data object if so configured', { plan: 3 }, (done) => {

const server = new Hapi.Server();
server.connection();

server.route({
method: 'GET',
path: '/',
handler: (request, reply) => {

request.log(['user', 'test'], 'you called the / route');
reply();
}
});

const out = new GoodReporter.Writer(true);
const monitor = internals.monitorFactory(server, {
includes: {
request: ['headers'],
response: []
},
reporters: { foo: [out] }
});

AsyncSeries([
server.start.bind(server),
monitor.start.bind(monitor),
(callback) => {

server.inject({
url: '/',
headers: {
'X-TraceID': 'ABCD12345'
}
}, (res) => {

expect(res.statusCode).to.equal(200);
setTimeout(() => {

expect(out.data).to.have.length(2);

const event = out.data[1];

expect(event.headers['x-traceid']).to.equal('ABCD12345');
server.stop(callback);
}, 50);
});
}
], done);
});

it('reports extension events when they occur', { plan: 14 }, (done) => {

const server = new Hapi.Server();
Expand Down
2 changes: 1 addition & 1 deletion test/utils.js
Expand Up @@ -101,7 +101,7 @@ describe('utils', () => {

it('can be stringifyed', { plan: 2 }, (done) => {

const err = new Utils.RequestError({
const err = new Utils.RequestError({}, {
id: 15,
url: 'http://localhost:9001',
method: 'PUT',
Expand Down

0 comments on commit 549662a

Please sign in to comment.