diff --git a/CHANGES.md b/CHANGES.md index 59bfe4d..b531bf3 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -12,6 +12,9 @@ ### Fix ### * #132: Handle multibyte characters properly in gzipped responses +### New ### +- #95: Add support for `before` and `after` hooks on client requests. + ## 1.5.2 - Switch back to restify-errors@3 to fix backward incompatiblity in diff --git a/README.md b/README.md index bfa8a90..40412f4 100644 --- a/README.md +++ b/README.md @@ -138,8 +138,10 @@ var client = restify.createJsonClient({ |Name | Type | Description | | :--- | :----: | :---- | |accept|String|Accept header to send| +|after|Function|Function for tracing. Run after every request, after the client has received the response.| |audit|Boolean|Enable Audit logging| |auditor|Function|Function for Audit logging| +|before|Function|Function for tracing. Run before every request.| |connectTimeout|Number|Amount of time to wait for a socket| |contentType|String|Content-Type header to send| |requestTimeout|Number|Amount of time to wait for the request to finish| @@ -488,6 +490,118 @@ Request timings are available under the `req.getTimings()` in milliseconds: All timings except `total` can be `null` under various circumstances like keep-alive connection, missing https etc. +## Tracing + +This module allows the caller to add hook functions which will be called before +and/or after each request from a JsonClient, StringClient or HttpClient. To +utilize these hooks, you can pass the before and/or after options when creating +the client. Any requests made by this client will then cause these functions to +be called. + +These hooks are useful for integrating with a tracing framework such as +[OpenTracing](http://opentracing.io/), as they allow passing in functions that +add tracing for all outgoing requests made by your application. + +If passed, the value of the `before` parameter should be a function with the +following prototype: + +``` +function before(opts, next) { +``` + +where `opts` is an object containing the options for the request, and `next` +is a callback to call when processing is complete. Some things you might want to +do in this before function include: + + * writing a trace log message indicating that you're making a request + * modifying opts.headers to include additional headers in the outbound request + +Once processing is complete, the `before` function *must* call the `next` +callback. If an object is passed as a parameter to `next` as in: + +``` +function before(opts, next) { + next({hello: 'world'}); +} +``` + +the object argument to next will be passed as an argument to the `after` +function. + +If passed, the value of the `after` parameter should be a function with the +following prototype: + +``` +function after(err, req, res, ctx, next) { +``` + +Where the parameters are: + +|Name | Type | Description | +| :--- | :----: | :---- | +|err|Error Object|The err object as returned by req.once('result', function (err, ...)| +|req|Request Object|The [req](http://restify.com/#request-api) object for this request| +|res|Response Object|The [res](http://restify.com/#response-api) object as returned by req.once('result', function (err, res)| +|ctx|Object|The object passed by the `before` hook to its callback| +|next|Function|The callback your handler must call when processing is complete| + +All of these parameters can also be `undefined` in cases where the value does +not make sense. For example the `ctx` parameter will be `undefined` if `before` +was not called or did not pass an object to its callback, and `req` and `res` +will be undefined in error cases where a request failed early on. + +The `after` caller is most useful for logging the fact that a request completed +and indicating errors or response details to a tracing system. + +A simple example of this tracing (missing any error checking) in action would be: + +``` +var restifyClients = require('restify-clients'); + +var client = restifyClients.createJsonClient({ + after: function _afterCb(err, req, res, ctx, next) { + console.error('AFTER: ' + JSON.stringify({ + ctx: ctx, + statusCode: res.statusCode + }, null, 2)); + next(); + }, before: function _beforeCb(opts, next) { + console.error('BEFORE: ' + JSON.stringify({ + headers: opts.headers + }, null, 2)); + // set an additional header before the request is made + opts.headers['x-example-header'] = 'exemplary'; + next({hello: 'from _beforeCb'}); + }, url: 'http://127.0.0.1:8080' +}); + +// make a GET request to /hello +client.get('/hello', function _getCb(err, req, res, obj) { + console.error('REQUEST COMPLETE'); +}); +``` + +which, if run, should output something like: + +``` +BEFORE: { + "headers": { + "accept": "application/json", + "user-agent": "restify/1.4.1 (x64-darwin; v8/4.5.103.37; OpenSSL/1.0.2j) node/4.6.0", + "date": "Fri, 04 Nov 2016 06:06:42 GMT" + } +} +AFTER: { + "ctx": { + "hello": "from _beforeCb" + }, + "statusCode": 200 +} +REQUEST COMPLETE +``` + +assuming you have a server listening on 127.0.0.1:8080 responding to `GET /hello`. + ## Contributing Add unit tests for any new or changed functionality. Ensure that lint and style diff --git a/lib/HttpClient.js b/lib/HttpClient.js index dbe637c..d755129 100644 --- a/lib/HttpClient.js +++ b/lib/HttpClient.js @@ -460,6 +460,17 @@ function HttpClient(options) { this.key = options.key; this.name = options.name || 'HttpClient'; + + // add before and after hooks if they're passed + + if (options.before) { + self._before = options.before; + } + + if (options.after) { + self._after = options.after; + } + this.passphrase = options.passphrase; this.pfx = options.pfx; this.query = options.query; @@ -710,19 +721,68 @@ HttpClient.prototype.basicAuth = function basicAuth(username, password) { return (this); }; +function dummyBeforeHook(opts, next) { + next(); +} + +/* eslint-disable handle-callback-err */ +function dummyAfterHook(err, req, res, ctx, next) { + next(); +} HttpClient.prototype.request = function request(opts, cb) { assert.object(opts, 'options'); assert.func(cb, 'callback'); + var _rawRequest = rawRequest; + var self = this; + /* eslint-disable no-param-reassign */ + if (self._before || self._after) { + _rawRequest = function _wrappedRawRequest(_opts, _cb) { + var _after = self._after || dummyAfterHook; + var _before = self._before || dummyBeforeHook; + + _before(opts, function _beforeCb(ctx) { + // Now call the original rawRequest but wrap the callback so we + // can hook in our after hook. + rawRequest(opts, function _rawRequestCb(err, req) { + // We've now attempted to make the request. It doesn't seem + // like there's a normal codepath where we get an `err` but + // no `req` because even when you set the connectTimeout so + // that happens immediately, it will still create a req. As + // such, we verify here that we always have a req object. + // + // We'll call our _after() hook when that req emits 'result' + // so that we have the 'res' parameter. + + assert.object(req, 'req'); + + req.once('result', function _onResult(resErr, resRes) { + _after(resErr, req, resRes, ctx, + function _afterCb() { + // When _after() calls its callback, we don't + // currently do anything because we don't want + // after to be able to delay requests. If we + // did want that, we could move the call to + // _cb() here. + } + ); + }); + + // Call the original callback intended for rawRequest() + _cb(err, req); + }); + }); + }; + } cb = once(cb); /* eslint-enable no-param-reassign */ opts.audit = this.audit; if (opts.retry === false) { - rawRequest(opts, cb); + _rawRequest(opts, cb); return; } @@ -730,7 +790,7 @@ HttpClient.prototype.request = function request(opts, cb) { var retry = cloneRetryOptions(opts.retry); opts._keep_alive = this._keep_alive; - call = backoff.call(rawRequest, opts, cb); + call = backoff.call(_rawRequest, opts, cb); call.setStrategy(new backoff.ExponentialStrategy({ initialDelay: retry.minTimeout, maxDelay: retry.maxTimeout diff --git a/test/e2e.js b/test/e2e.js index 90f91cc..7c4e11c 100644 --- a/test/e2e.js +++ b/test/e2e.js @@ -8,8 +8,8 @@ var clients = require('../lib'); describe('restify-client tests against real web server', function () { it('have timings', function (done) { - var client = clients.createJsonClient({ - url: 'https://netflix.com' + var client = clients.createStringClient({ + url: 'https://www.netflix.com' }); client.get('/', function (err, req, res) { @@ -24,6 +24,7 @@ describe('restify-client tests against real web server', function () { assert.isNumber(timings.firstByte); assert.isNumber(timings.contentTransfer); assert.isNumber(timings.total); + client.close(); done(); }); }); diff --git a/test/index.js b/test/index.js index bebd552..8ae3153 100644 --- a/test/index.js +++ b/test/index.js @@ -163,6 +163,14 @@ function sendJsonString(req, res, next) { return next(); } +// sends back a body with the contents of the restify-clients-test-header +// received +function sendTestHeader(req, res, next) { + res.header('content-type', 'json'); + res.send(200, req.header('restify-clients-test-header', 'missing')); + next(); +} + function getLog(name, stream, level) { return (bunyan.createLogger({ level: (process.env.LOG_LEVEL || level || 'fatal'), @@ -1424,4 +1432,163 @@ describe('restify-client tests', function () { }); }); }); + + // Ensure the .before and .after functions are called for each client. + describe('before/after hooks', function () { + var _clients = { + HttpClient: { + creator: clients.createHttpClient + }, JsonClient: { + creator: clients.createJsonClient + }, StringClient: { + creator: clients.createStringClient + } + }; + var handles = []; + var server = restify.createServer({}); + var serverUrl; + + function _getBody(clientType, req, data, cb) { + if (clientType === 'HttpClient') { + req.on('result', function (err, res) { + var body = ''; + + assert.ifError(err); + + res.setEncoding('utf8'); + res.on('data', function (chunk) { + body += chunk; + }); + + res.on('end', function () { + cb(body.replace(/"/g, '')); + }); + }); + } else { + cb(data.replace(/"/g, '')); + } + } + + before(function (callback) { + var url = 'http://127.0.0.1'; + + server.get('/testheader', sendTestHeader); + server.listen(); + serverUrl = url + ':' + server.address().port; + + callback(); + }); + + // {Http,Json,String}Client should be able to use .before and .after + Object.keys(_clients).forEach(function (clientName) { + var _client = _clients[clientName]; + + it('request from ' + clientName, function (done) { + var afterCtx = false; + var afterRan = false; + var beforeRan = false; + var client; + + client = _client.creator({ + after: function (err, req, res, ctx, cb) { + assert.ifError(err); + afterRan = true; + // got ctx from before() function + if (ctx && ctx.hello === 'world') { + afterCtx = true; + } + cb(); + }, before: function (opts, cb) { + beforeRan = true; + opts.headers['restify-clients-test-header'] = + clientName; + cb({hello: 'world'}); + }, url: serverUrl + }); + + handles.push(client); + + client.get('/testheader', function (err, req, res, data) { + assert.ifError(err); + _getBody(clientName, req, data, function (body) { + assert.equal(body, clientName); + assert.equal(beforeRan, true); + assert.equal(afterRan, true); + assert.equal(afterCtx, true); + + done(); + }); + }); + }); + + it('request (w/o after) from ' + clientName, function (done) { + var afterRan = false; + var beforeRan = false; + var client; + + client = _client.creator({ + before: function (opts, cb) { + beforeRan = true; + opts.headers['restify-clients-test-header'] = + clientName; + cb({hello: 'world'}); + }, url: serverUrl + }); + + handles.push(client); + + client.get('/testheader', function (err, req, res, data) { + assert.ifError(err); + _getBody(clientName, req, data, function (body) { + assert.equal(body, clientName); + assert.equal(beforeRan, true); + assert.equal(afterRan, false); + + done(); + }); + }); + }); + + it('request (w/o before) from ' + clientName, function (done) { + var afterCtx = false; + var afterRan = false; + var beforeRan = false; + var client; + + client = _client.creator({ + after: function (err, req, res, ctx, cb) { + assert.ifError(err); + afterRan = true; + // got ctx from before() function + if (ctx && ctx.hello === 'world') { + afterCtx = true; + } + cb(); + }, url: serverUrl + }); + + handles.push(client); + + client.get('/testheader', function (err, req, res, data) { + assert.ifError(err); + _getBody(clientName, req, data, function (body) { + assert.equal(body, 'missing'); + assert.equal(beforeRan, false); + assert.equal(afterRan, true); + assert.equal(afterCtx, false); + + done(); + }); + }); + }); + }); + + after(function (callback) { + handles.forEach(function (clientHandle) { + clientHandle.close(); + }); + server.close(); + callback(); + }); + }); });