Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
feat: deprecate req.closed
Node.js added `closed` getter-only property to streams. This commit
deprecates req.closed to avoid monkey-patching something that might be
used in Node.js core or user-land libraries.
`req.connectionState() === 'close'` can be used to achieve the same
result moving forward. req.closed will be removed on Restify 10 to open
way to support Node.js v18.
  • Loading branch information
mmarchini committed Nov 15, 2022
1 parent 839fb4a commit d052b7c
Show file tree
Hide file tree
Showing 7 changed files with 66 additions and 33 deletions.
2 changes: 1 addition & 1 deletion lib/chain.js
Expand Up @@ -140,7 +140,7 @@ Chain.prototype.run = function run(req, res, done) {
var handler = self._stack[index++];

// all done or request closed
if (!handler || req.closed()) {
if (!handler || req.connectionState() === 'close') {
process.nextTick(function nextTick() {
return done(err, req, res);
});
Expand Down
5 changes: 4 additions & 1 deletion lib/plugins/static.js
Expand Up @@ -93,7 +93,10 @@ function serveStatic(options) {
var re = new RegExp('^' + escapeRE(p) + '/?.*');

function serveFileFromStats(file, err, stats, isGzip, req, res, next) {
if (typeof req.closed === 'function' && req.closed()) {
if (
typeof req.connectionState === 'function' &&
req.connectionState() === 'close'
) {
next(false);
return;
}
Expand Down
6 changes: 6 additions & 0 deletions lib/request.js
Expand Up @@ -2,6 +2,7 @@

'use strict';

const { emitWarning } = require('node:process');
var url = require('url');
var sprintf = require('util').format;

Expand Down Expand Up @@ -846,6 +847,11 @@ function patch(Request) {
* @returns {Boolean} is closed
*/
Request.prototype.closed = function closed() {
emitWarning(
'restify req.closed is deprecated, will be removed on Restify 10',
'RestifyDeprecationWarning',
'RestifyDEPReqClosed'
);
var self = this;
return self.connectionState() === 'close';
};
Expand Down
50 changes: 25 additions & 25 deletions test/chain.test.js
Expand Up @@ -29,8 +29,8 @@ test('calls all the handlers', function(t) {
{
startHandlerTimer: function() {},
endHandlerTimer: function() {},
closed: function() {
return false;
connectionState: function() {
return '';
}
},
{},
Expand Down Expand Up @@ -58,8 +58,8 @@ test('abort with Error in next', function(t) {
{
startHandlerTimer: function() {},
endHandlerTimer: function() {},
closed: function() {
return false;
connectionState: function() {
return '';
}
},
{},
Expand All @@ -85,7 +85,7 @@ test('abort with false in next', function(t) {
{
startHandlerTimer: function() {},
endHandlerTimer: function() {},
closed: function() {
connectionState: function() {
return false;
}
},
Expand All @@ -112,8 +112,8 @@ test('abort with closed request', function(t) {
{
startHandlerTimer: function() {},
endHandlerTimer: function() {},
closed: function() {
return closed;
connectionState: function() {
return closed ? 'close' : '';
}
},
{},
Expand Down Expand Up @@ -143,8 +143,8 @@ test('cals error middleware', function(t) {
{
startHandlerTimer: function() {},
endHandlerTimer: function() {},
closed: function() {
return false;
connectionState: function() {
return '';
}
},
{},
Expand All @@ -170,8 +170,8 @@ test('onceNext prevents double next calls', function(t) {
{
startHandlerTimer: function() {},
endHandlerTimer: function() {},
closed: function() {
return false;
connectionState: function() {
return '';
}
},
{},
Expand Down Expand Up @@ -208,8 +208,8 @@ test('throws error for double next calls in strictNext mode', function(t) {
{
startHandlerTimer: function() {},
endHandlerTimer: function() {},
closed: function() {
return false;
connectionState: function() {
return '';
}
},
{},
Expand All @@ -234,8 +234,8 @@ test('calls req.startHandlerTimer', function(t) {
t.done();
},
endHandlerTimer: function() {},
closed: function() {
return false;
connectionState: function() {
return '';
}
},
{},
Expand All @@ -257,8 +257,8 @@ test('calls req.endHandlerTimer', function(t) {
t.equal(handleName, 'foo');
t.done();
},
closed: function() {
return false;
connectionState: function() {
return '';
}
},
{},
Expand Down Expand Up @@ -299,8 +299,8 @@ test('waits async handlers', function(t) {
{
startHandlerTimer: function() {},
endHandlerTimer: function() {},
closed: function() {
return false;
connectionState: function() {
return '';
}
},
{},
Expand Down Expand Up @@ -329,8 +329,8 @@ test('abort with rejected promise', function(t) {
{
startHandlerTimer: function() {},
endHandlerTimer: function() {},
closed: function() {
return false;
connectionState: function() {
return '';
}
},
{},
Expand Down Expand Up @@ -359,8 +359,8 @@ test('abort with rejected promise without error', function(t) {
{
startHandlerTimer: function() {},
endHandlerTimer: function() {},
closed: function() {
return false;
connectionState: function() {
return '';
},
path: function() {
return '/';
Expand Down Expand Up @@ -395,8 +395,8 @@ test('abort with throw inside async function', function(t) {
{
startHandlerTimer: function() {},
endHandlerTimer: function() {},
closed: function() {
return false;
connectionState: function() {
return '';
}
},
{},
Expand Down
8 changes: 4 additions & 4 deletions test/chainComposer.test.js
Expand Up @@ -29,8 +29,8 @@ test('chainComposer creates a valid chain for a handler array ', function(t) {
{
startHandlerTimer: function() {},
endHandlerTimer: function() {},
closed: function() {
return false;
connectionState: function() {
return '';
}
},
{},
Expand All @@ -53,8 +53,8 @@ test('chainComposer creates a valid chain for a single handler', function(t) {
{
startHandlerTimer: function() {},
endHandlerTimer: function() {},
closed: function() {
return false;
connectionState: function() {
return '';
}
},
{},
Expand Down
24 changes: 24 additions & 0 deletions test/request.test.js
Expand Up @@ -275,3 +275,27 @@ test('should emit restifyDone event when request is fully served with error', fu
clientDone = true;
});
});

test('should emit warning if closed is called', function(t) {
let warningCalled = false;
SERVER.get('/ping/:name', function(req, res, next) {
function testWarning(warning) {
t.equal(warning.name, 'RestifyDeprecationWarning');
t.equal(warning.code, 'RestifyDEPReqClosed');
t.ok(warning.stack);
warningCalled = true;

res.send('ok');
return next();
}
process.once('warning', testWarning);
t.notOk(req.closed());
});

CLIENT.get('/ping/lagavulin', function(err, _, res) {
t.ifError(err);
t.equal(res.statusCode, 200);
t.ok(warningCalled);
t.end();
});
});
4 changes: 2 additions & 2 deletions test/router.test.js
Expand Up @@ -16,8 +16,8 @@ var helper = require('./lib/helper.js');
var test = helper.test;
var mockReq = {
params: {},
closed: function() {
return false;
connectionState: function() {
return '';
},
startHandlerTimer: function() {},
endHandlerTimer: function() {}
Expand Down

0 comments on commit d052b7c

Please sign in to comment.