diff --git a/lib/plugins/metrics.js b/lib/plugins/metrics.js index b7b5121d5..eddf7f165 100644 --- a/lib/plugins/metrics.js +++ b/lib/plugins/metrics.js @@ -47,12 +47,8 @@ function createMetrics(opts, callback) { // connection state can currently only have the following values: // 'close' | 'aborted' | undefined. // - // it is possible to get a 200 statusCode with a connectionState - // value of 'close' or 'aborted'. i.e., the client timed out, - // but restify thinks it "sent" a response. connectionState should - // always be the primary source of truth here, and check it first - // before consuming statusCode. otherwise, it may result in skewed - // metrics. + // if the connection state is 'close' or 'aborted' + // the status code will be set to 444 connectionState: req.connectionState && req.connectionState(), unfinishedRequests: opts.server.inflightRequests && opts.server.inflightRequests(), diff --git a/lib/request.js b/lib/request.js index aca11371b..47ed10016 100644 --- a/lib/request.js +++ b/lib/request.js @@ -826,7 +826,7 @@ function patch(Request) { * @memberof Request * @instance * @function connectionState - * @returns {String} connection state (`"closed"`, `"aborted"`) + * @returns {String} connection state (`"close"`, `"aborted"`) */ Request.prototype.connectionState = function connectionState() { var self = this; diff --git a/lib/response.js b/lib/response.js index 0d4d69b76..0b9c355d4 100644 --- a/lib/response.js +++ b/lib/response.js @@ -380,6 +380,14 @@ function patch(Response) { // Now lets try to derive values for optional arguments that we were not // provided, otherwise we choose sane defaults. + // Request was aborted or closed. Override the status code + if ( + self.req.connectionState() === 'close' || + self.req.connectionState() === 'aborted' + ) { + code = 444; + } + // If the body is an error object and we were not given a status code, // try to derive it from the error object, otherwise default to 500 if (!code && body instanceof Error) { diff --git a/test/plugins/audit.test.js b/test/plugins/audit.test.js index a378e4743..9332b71d8 100644 --- a/test/plugins/audit.test.js +++ b/test/plugins/audit.test.js @@ -605,4 +605,74 @@ describe('audit logger', function() { assert.ifError(err); }); }); + + it('should log 444 status code for aborted request', function(done) { + SERVER.once( + 'after', + restify.plugins.auditLogger({ + log: bunyan.createLogger({ + name: 'audit', + streams: [ + { + level: 'info', + stream: process.stdout + } + ] + }), + server: SERVER, + event: 'after' + }) + ); + + SERVER.once('audit', function(data) { + assert.ok(data); + assert.ok(data.req_id); + assert.isNumber(data.latency); + assert.equal(444, data.res.statusCode); + done(); + }); + + SERVER.get('/audit', function(req, res, next) { + req.emit('aborted'); + res.send(); + next(); + }); + + CLIENT.get('/audit', function(err, req, res) {}); + }); + + it('should log 444 for closed request', function(done) { + SERVER.once( + 'after', + restify.plugins.auditLogger({ + log: bunyan.createLogger({ + name: 'audit', + streams: [ + { + level: 'info', + stream: process.stdout + } + ] + }), + server: SERVER, + event: 'after' + }) + ); + + SERVER.once('audit', function(data) { + assert.ok(data); + assert.ok(data.req_id); + assert.isNumber(data.latency); + assert.equal(444, data.res.statusCode); + done(); + }); + + SERVER.get('/audit', function(req, res, next) { + req.emit('close'); + res.send(); + next(); + }); + + CLIENT.get('/audit', function(err, req, res) {}); + }); }); diff --git a/test/plugins/metrics.test.js b/test/plugins/metrics.test.js index 572774a1e..5795fcb0b 100644 --- a/test/plugins/metrics.test.js +++ b/test/plugins/metrics.test.js @@ -96,7 +96,7 @@ describe('request metrics plugin', function() { assert.equal(err.name, 'RequestCloseError'); assert.isObject(metrics, 'metrics'); - assert.equal(metrics.statusCode, 202); + assert.equal(metrics.statusCode, 444); assert.isAtLeast(metrics.latency, 200); assert.equal(metrics.path, '/foo'); assert.equal(metrics.method, 'GET'); @@ -137,7 +137,7 @@ describe('request metrics plugin', function() { assert.equal(err.name, 'RequestAbortedError'); assert.isObject(metrics, 'metrics'); - assert.equal(metrics.statusCode, 202); + assert.equal(metrics.statusCode, 444); assert.isAtLeast(metrics.latency, 200); assert.equal(metrics.path, '/foo'); assert.equal(metrics.method, 'GET'); @@ -155,7 +155,6 @@ describe('request metrics plugin', function() { }); CLIENT.get('/foo?a=1', function(err, _, res) { - assert.ifError(err); return done(); }); });