Skip to content

Commit 2b3369c

Browse files
fernandolguevarafernandolguevara
committed
refactor(response-proxy): match nodejs behavior
1 parent 2991c82 commit 2b3369c

File tree

2 files changed

+215
-30
lines changed

2 files changed

+215
-30
lines changed

‎index.js

Lines changed: 51 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@ var debug = require('debug')('compression')
2222
var onHeaders = require('on-headers')
2323
var vary = require('vary')
2424
var zlib = require('zlib')
25+
var isUint8Array = require('@stdlib/assert-is-uint8array')
26+
const { ServerResponse } = require('http')
2527

2628
/**
2729
* Module exports.
@@ -56,6 +58,8 @@ function compression (options) {
5658
threshold = 1024
5759
}
5860

61+
function noop () { }
62+
5963
return function compression (req, res, next) {
6064
var ended = false
6165
var length
@@ -75,40 +79,69 @@ function compression (options) {
7579

7680
// proxy
7781

78-
res.write = function write (chunk, encoding, cb) {
79-
if (ended) {
80-
return false
82+
res.write = function write (chunk, encoding, callback) {
83+
if (chunk === null) {
84+
// throw ERR_STREAM_NULL_VALUES
85+
return _write.call(this, chunk, encoding, callback)
86+
} else if (typeof chunk === 'string' || isUint8Array(chunk)) {
87+
// noop
88+
} else {
89+
// throw ERR_INVALID_ARG_TYPE
90+
return _write.call(this, chunk, encoding, callback)
8191
}
8292

83-
if (!cb && typeof encoding === 'function') {
84-
cb = encoding
93+
if (!callback && typeof encoding === 'function') {
94+
callback = encoding
8595
encoding = undefined
8696
}
8797

98+
if (typeof callback !== 'function') {
99+
callback = noop
100+
}
101+
102+
if (res.destroyed || res.finished || ended) {
103+
// HACK: node doesn't expose internal errors,
104+
// we need to fake response to throw underlying errors type
105+
var fakeRes = new ServerResponse({})
106+
if (!res.destroyed) {
107+
fakeRes.destroyed = fakeRes.finished = true
108+
} else {
109+
fakeRes.destroyed = true
110+
}
111+
// throw ERR_STREAM_DESTROYED or ERR_STREAM_WRITE_AFTER_END
112+
return _write.call(fakeRes, chunk, encoding, callback)
113+
}
114+
88115
if (!this._header) {
89116
this._implicitHeader()
90117
}
91118

92119
return stream
93-
? stream.write(toBuffer(chunk, encoding), encoding, cb)
94-
: _write.call(this, chunk, encoding, cb)
120+
? stream.write(toBuffer(chunk, encoding), encoding, callback)
121+
: _write.call(this, chunk, encoding, callback)
95122
}
96123

97-
res.end = function end (chunk, encoding, cb) {
98-
if (ended) {
99-
return false
100-
}
101-
102-
if (!cb) {
124+
res.end = function end (chunk, encoding, callback) {
125+
if (!callback) {
103126
if (typeof chunk === 'function') {
104-
cb = chunk
127+
callback = chunk
105128
chunk = encoding = undefined
106129
} else if (typeof encoding === 'function') {
107-
cb = encoding
130+
callback = encoding
108131
encoding = undefined
109132
}
110133
}
111134

135+
if (typeof callback !== 'function') {
136+
callback = noop
137+
}
138+
139+
if (this.destroyed || this.finished || ended) {
140+
this.finished = ended
141+
// throw ERR_STREAM_WRITE_AFTER_END or ERR_STREAM_ALREADY_FINISHED
142+
return _end.call(this, chunk, encoding, callback)
143+
}
144+
112145
if (!this._header) {
113146
// estimate the length
114147
if (!this.getHeader('Content-Length')) {
@@ -119,16 +152,16 @@ function compression (options) {
119152
}
120153

121154
if (!stream) {
122-
return _end.call(this, chunk, encoding, cb)
155+
return _end.call(this, chunk, encoding, callback)
123156
}
124157

125158
// mark ended
126159
ended = true
127160

128161
// write Buffer for Node.js 0.8
129162
return chunk
130-
? stream.end(toBuffer(chunk, encoding), encoding, cb)
131-
: stream.end(cb)
163+
? stream.end(toBuffer(chunk, encoding), encoding, callback)
164+
: stream.end(callback)
132165
}
133166

134167
res.on = function on (type, listener) {

‎test/compression.js

Lines changed: 164 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -6,21 +6,136 @@ var crypto = require('crypto')
66
var http = require('http')
77
var request = require('supertest')
88
var zlib = require('zlib')
9+
var sinon = require('sinon')
910

1011
var compression = require('..')
1112

1213
describe('compression()', function () {
13-
it('request should end', function (done) {
14-
var reponseText = 'hello, world'
14+
describe('should work with valid types (string, Buffer, Uint8Array)', function () {
15+
it('res.write(string)', function (done) {
16+
var server = createServer({ threshold: 0 }, function (req, res) {
17+
res.setHeader('Content-Type', 'text/plain')
18+
res.end('hello world')
19+
})
20+
21+
request(server)
22+
.get('/')
23+
.set('Accept-Encoding', 'gzip')
24+
.expect('Content-Encoding', 'gzip')
25+
.expect(200, done)
26+
})
27+
28+
it('res.write(Buffer)', function (done) {
29+
var server = createServer({ threshold: 0 }, function (req, res) {
30+
res.setHeader('Content-Type', 'text/plain')
31+
res.end(Buffer.from('hello world'))
32+
})
33+
34+
request(server)
35+
.get('/')
36+
.set('Accept-Encoding', 'gzip')
37+
.expect('Content-Encoding', 'gzip')
38+
.expect(200, done)
39+
})
40+
41+
it('res.write(Uint8Array)', function (done) {
42+
var server = createServer({ threshold: 0 }, function (req, res) {
43+
res.setHeader('Content-Type', 'text/plain')
44+
res.end(new Uint8Array(1))
45+
})
46+
47+
request(server)
48+
.get('/')
49+
.set('Accept-Encoding', 'gzip')
50+
.expect('Content-Encoding', 'gzip')
51+
.expect(200, done)
52+
})
53+
})
54+
55+
describe('should throw with invalid types', function () {
56+
it('res.write(1) should fire ERR_INVALID_ARG_TYPE', function (done) {
57+
var server = createServer({ threshold: 0 }, function (req, res) {
58+
res.setHeader('Content-Type', 'text/plain')
59+
try {
60+
res.write(1)
61+
} catch (err) {
62+
assert.ok(err.code === 'ERR_INVALID_ARG_TYPE')
63+
res.statusCode = 500
64+
res.flush()
65+
res.end()
66+
}
67+
})
68+
69+
request(server)
70+
.get('/')
71+
.set('Accept-Encoding', 'gzip')
72+
.expect(500, done)
73+
})
74+
75+
it('res.write({}) should fire ERR_INVALID_ARG_TYPE', function (done) {
76+
var server = createServer({ threshold: 0 }, function (req, res) {
77+
res.setHeader('Content-Type', 'text/plain')
78+
try {
79+
res.write({})
80+
} catch (err) {
81+
assert.ok(err.code === 'ERR_INVALID_ARG_TYPE')
82+
res.statusCode = 500
83+
res.flush()
84+
res.end()
85+
}
86+
})
87+
88+
request(server)
89+
.get('/')
90+
.set('Accept-Encoding', 'gzip')
91+
.expect(500, done)
92+
})
1593

94+
it('res.write(null) should fire ERR_STREAM_NULL_VALUES', function (done) {
95+
var server = createServer({ threshold: 0 }, function (req, res) {
96+
res.setHeader('Content-Type', 'text/plain')
97+
try {
98+
res.write(null)
99+
} catch (err) {
100+
assert.ok(err.code === 'ERR_STREAM_NULL_VALUES')
101+
res.statusCode = 500
102+
res.flush()
103+
res.end()
104+
}
105+
})
106+
107+
request(server)
108+
.get('/')
109+
.set('Accept-Encoding', 'gzip')
110+
.expect(500, done)
111+
})
112+
})
113+
114+
it('res.write() should throw ERR_STREAM_DESTROYED when destoyed stream', function (done) {
16115
var server = createServer({ threshold: 0 }, function (req, res) {
17116
res.setHeader('Content-Type', 'text/plain')
117+
res.end('hello world')
18118

19-
res.write(Buffer.from(reponseText), (err) => {
20-
if (err) {
21-
console.error('err', err)
22-
}
119+
server.on('close', () => {
120+
res.write('hello world', function (err) {
121+
assert.ok(err.code === 'ERR_STREAM_DESTROYED')
122+
})
123+
})
124+
})
125+
126+
request(server)
127+
.get('/')
128+
.set('Accept-Encoding', 'gzip')
129+
.expect(shouldHaveHeader('Content-Encoding'))
130+
.expect(shouldHaveBodyLength('hello world'.length))
131+
.expect(200, done)
132+
})
23133

134+
it('res.write() should call callback if passsed', function (done) {
135+
var server = createServer({ threshold: 0 }, function (req, res) {
136+
res.setHeader('Content-Type', 'text/plain')
137+
138+
res.write('hello, world', function () {
24139
res.end()
25140
})
26141
})
@@ -29,10 +144,32 @@ describe('compression()', function () {
29144
.get('/')
30145
.set('Accept-Encoding', 'gzip')
31146
.expect(shouldHaveHeader('Content-Encoding'))
32-
.expect(shouldHaveBodyLength(reponseText.length))
147+
.expect(shouldHaveBodyLength('hello, world'.length))
33148
.expect(200, done)
34149
})
35150

151+
it('res.write() should call callback with error after end', function (done) {
152+
var onError = sinon.spy(function (err) {
153+
assert.ok(err.code === 'ERR_STREAM_WRITE_AFTER_END')
154+
})
155+
156+
var server = createServer({ threshold: 0 }, function (req, res) {
157+
res.setHeader('Content-Type', 'text/plain')
158+
res.end()
159+
160+
res.write('hello, world', onError)
161+
162+
process.nextTick(function () {
163+
assert.ok(onError.callCount > 0)
164+
})
165+
})
166+
167+
request(server)
168+
.get('/')
169+
.set('Accept-Encoding', 'gzip')
170+
.end(done)
171+
})
172+
36173
it('should skip HEAD', function (done) {
37174
var server = createServer({ threshold: 0 }, function (req, res) {
38175
res.setHeader('Content-Type', 'text/plain')
@@ -460,17 +597,28 @@ describe('compression()', function () {
460597
})
461598

462599
it('should return false writing after end', function (done) {
600+
var onError = sinon.spy(function (err) {
601+
assert.ok(err.code === 'ERR_STREAM_WRITE_AFTER_END')
602+
})
603+
463604
var server = createServer({ threshold: 0 }, function (req, res) {
605+
res.on('error', onError)
464606
res.setHeader('Content-Type', 'text/plain')
607+
465608
res.end('hello, world')
466-
assert.ok(res.write() === false)
467-
assert.ok(res.end() === false)
609+
610+
assert.ok(res.write('', onError) === false)
611+
612+
process.nextTick(function () {
613+
assert.ok(onError.callCount > 0)
614+
})
468615
})
469616

470617
request(server)
471618
.get('/')
472619
.set('Accept-Encoding', 'gzip')
473-
.expect('Content-Encoding', 'gzip', done)
620+
.expect('Content-Encoding', 'gzip')
621+
.end(done)
474622
})
475623
})
476624

@@ -682,9 +830,12 @@ describe('compression()', function () {
682830
})
683831
})
684832

685-
function createServer (opts, fn) {
833+
function createServer (opts, fn, t) {
686834
var _compression = compression(opts)
687835
return http.createServer(function (req, res) {
836+
if (t) {
837+
res.on('finish', function () { console.log(t.title, 'server closed') })
838+
}
688839
_compression(req, res, function (err) {
689840
if (err) {
690841
res.statusCode = err.status || 500
@@ -705,7 +856,8 @@ function shouldHaveBodyLength (length) {
705856

706857
function shouldHaveHeader (header) {
707858
return function (res) {
708-
assert.ok((header.toLowerCase() in res.headers), 'should have header ' + header)
859+
var ok = (header.toLowerCase() in res.headers)
860+
assert.ok(ok, 'should have header ' + header)
709861
}
710862
}
711863

0 commit comments

Comments
 (0)