Skip to content

Commit d052b7c

Browse files
mmarchinimmarchini
committed
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.
1 parent 839fb4a commit d052b7c

File tree

7 files changed

+66
-33
lines changed

7 files changed

+66
-33
lines changed

‎lib/chain.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,7 @@ Chain.prototype.run = function run(req, res, done) {
140140
var handler = self._stack[index++];
141141

142142
// all done or request closed
143-
if (!handler || req.closed()) {
143+
if (!handler || req.connectionState() === 'close') {
144144
process.nextTick(function nextTick() {
145145
return done(err, req, res);
146146
});

‎lib/plugins/static.js

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,10 @@ function serveStatic(options) {
9393
var re = new RegExp('^' + escapeRE(p) + '/?.*');
9494

9595
function serveFileFromStats(file, err, stats, isGzip, req, res, next) {
96-
if (typeof req.closed === 'function' && req.closed()) {
96+
if (
97+
typeof req.connectionState === 'function' &&
98+
req.connectionState() === 'close'
99+
) {
97100
next(false);
98101
return;
99102
}

‎lib/request.js

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
'use strict';
44

5+
const { emitWarning } = require('node:process');
56
var url = require('url');
67
var sprintf = require('util').format;
78

@@ -846,6 +847,11 @@ function patch(Request) {
846847
* @returns {Boolean} is closed
847848
*/
848849
Request.prototype.closed = function closed() {
850+
emitWarning(
851+
'restify req.closed is deprecated, will be removed on Restify 10',
852+
'RestifyDeprecationWarning',
853+
'RestifyDEPReqClosed'
854+
);
849855
var self = this;
850856
return self.connectionState() === 'close';
851857
};

‎test/chain.test.js

Lines changed: 25 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,8 @@ test('calls all the handlers', function(t) {
2929
{
3030
startHandlerTimer: function() {},
3131
endHandlerTimer: function() {},
32-
closed: function() {
33-
return false;
32+
connectionState: function() {
33+
return '';
3434
}
3535
},
3636
{},
@@ -58,8 +58,8 @@ test('abort with Error in next', function(t) {
5858
{
5959
startHandlerTimer: function() {},
6060
endHandlerTimer: function() {},
61-
closed: function() {
62-
return false;
61+
connectionState: function() {
62+
return '';
6363
}
6464
},
6565
{},
@@ -85,7 +85,7 @@ test('abort with false in next', function(t) {
8585
{
8686
startHandlerTimer: function() {},
8787
endHandlerTimer: function() {},
88-
closed: function() {
88+
connectionState: function() {
8989
return false;
9090
}
9191
},
@@ -112,8 +112,8 @@ test('abort with closed request', function(t) {
112112
{
113113
startHandlerTimer: function() {},
114114
endHandlerTimer: function() {},
115-
closed: function() {
116-
return closed;
115+
connectionState: function() {
116+
return closed ? 'close' : '';
117117
}
118118
},
119119
{},
@@ -143,8 +143,8 @@ test('cals error middleware', function(t) {
143143
{
144144
startHandlerTimer: function() {},
145145
endHandlerTimer: function() {},
146-
closed: function() {
147-
return false;
146+
connectionState: function() {
147+
return '';
148148
}
149149
},
150150
{},
@@ -170,8 +170,8 @@ test('onceNext prevents double next calls', function(t) {
170170
{
171171
startHandlerTimer: function() {},
172172
endHandlerTimer: function() {},
173-
closed: function() {
174-
return false;
173+
connectionState: function() {
174+
return '';
175175
}
176176
},
177177
{},
@@ -208,8 +208,8 @@ test('throws error for double next calls in strictNext mode', function(t) {
208208
{
209209
startHandlerTimer: function() {},
210210
endHandlerTimer: function() {},
211-
closed: function() {
212-
return false;
211+
connectionState: function() {
212+
return '';
213213
}
214214
},
215215
{},
@@ -234,8 +234,8 @@ test('calls req.startHandlerTimer', function(t) {
234234
t.done();
235235
},
236236
endHandlerTimer: function() {},
237-
closed: function() {
238-
return false;
237+
connectionState: function() {
238+
return '';
239239
}
240240
},
241241
{},
@@ -257,8 +257,8 @@ test('calls req.endHandlerTimer', function(t) {
257257
t.equal(handleName, 'foo');
258258
t.done();
259259
},
260-
closed: function() {
261-
return false;
260+
connectionState: function() {
261+
return '';
262262
}
263263
},
264264
{},
@@ -299,8 +299,8 @@ test('waits async handlers', function(t) {
299299
{
300300
startHandlerTimer: function() {},
301301
endHandlerTimer: function() {},
302-
closed: function() {
303-
return false;
302+
connectionState: function() {
303+
return '';
304304
}
305305
},
306306
{},
@@ -329,8 +329,8 @@ test('abort with rejected promise', function(t) {
329329
{
330330
startHandlerTimer: function() {},
331331
endHandlerTimer: function() {},
332-
closed: function() {
333-
return false;
332+
connectionState: function() {
333+
return '';
334334
}
335335
},
336336
{},
@@ -359,8 +359,8 @@ test('abort with rejected promise without error', function(t) {
359359
{
360360
startHandlerTimer: function() {},
361361
endHandlerTimer: function() {},
362-
closed: function() {
363-
return false;
362+
connectionState: function() {
363+
return '';
364364
},
365365
path: function() {
366366
return '/';
@@ -395,8 +395,8 @@ test('abort with throw inside async function', function(t) {
395395
{
396396
startHandlerTimer: function() {},
397397
endHandlerTimer: function() {},
398-
closed: function() {
399-
return false;
398+
connectionState: function() {
399+
return '';
400400
}
401401
},
402402
{},

‎test/chainComposer.test.js

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,8 @@ test('chainComposer creates a valid chain for a handler array ', function(t) {
2929
{
3030
startHandlerTimer: function() {},
3131
endHandlerTimer: function() {},
32-
closed: function() {
33-
return false;
32+
connectionState: function() {
33+
return '';
3434
}
3535
},
3636
{},
@@ -53,8 +53,8 @@ test('chainComposer creates a valid chain for a single handler', function(t) {
5353
{
5454
startHandlerTimer: function() {},
5555
endHandlerTimer: function() {},
56-
closed: function() {
57-
return false;
56+
connectionState: function() {
57+
return '';
5858
}
5959
},
6060
{},

‎test/request.test.js

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -275,3 +275,27 @@ test('should emit restifyDone event when request is fully served with error', fu
275275
clientDone = true;
276276
});
277277
});
278+
279+
test('should emit warning if closed is called', function(t) {
280+
let warningCalled = false;
281+
SERVER.get('/ping/:name', function(req, res, next) {
282+
function testWarning(warning) {
283+
t.equal(warning.name, 'RestifyDeprecationWarning');
284+
t.equal(warning.code, 'RestifyDEPReqClosed');
285+
t.ok(warning.stack);
286+
warningCalled = true;
287+
288+
res.send('ok');
289+
return next();
290+
}
291+
process.once('warning', testWarning);
292+
t.notOk(req.closed());
293+
});
294+
295+
CLIENT.get('/ping/lagavulin', function(err, _, res) {
296+
t.ifError(err);
297+
t.equal(res.statusCode, 200);
298+
t.ok(warningCalled);
299+
t.end();
300+
});
301+
});

‎test/router.test.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,8 @@ var helper = require('./lib/helper.js');
1616
var test = helper.test;
1717
var mockReq = {
1818
params: {},
19-
closed: function() {
20-
return false;
19+
connectionState: function() {
20+
return '';
2121
},
2222
startHandlerTimer: function() {},
2323
endHandlerTimer: function() {}

0 commit comments

Comments
 (0)