From 47aa1a4c50b31d598e6b155feffaa0403ac304bd Mon Sep 17 00:00:00 2001 From: LavrovArtem Date: Tue, 15 Nov 2016 12:56:46 +0300 Subject: [PATCH] fix `Server shutdown with "Error: Can't set headers after they are sent." ` (close #937) --- .../destination-request/index.js | 18 ++++---- .../code-instrumentation/getters-test.js | 4 ++ test/server/proxy-test.js | 42 +++++++++++++++++++ 3 files changed, 56 insertions(+), 8 deletions(-) diff --git a/src/request-pipeline/destination-request/index.js b/src/request-pipeline/destination-request/index.js index 8e13d1efb..2fbe332e6 100644 --- a/src/request-pipeline/destination-request/index.js +++ b/src/request-pipeline/destination-request/index.js @@ -87,9 +87,12 @@ export default class DestinationRequest extends EventEmitter { this._send(requiresResBody(res)); } - _abort () { - this.aborted = true; - this.req.abort(); + _fatalError (msg) { + if (!this.aborted) { + this.aborted = true; + this.req.abort(); + this.emit('fatalError', getText(msg, this.opts.url)); + } } _isDNSErr (err) { @@ -100,10 +103,8 @@ export default class DestinationRequest extends EventEmitter { _onTimeout () { // NOTE: this handler is also called if we get an error response (for example, 404). So, we should check // for the response presence before raising the timeout error. - if (!this.hasResponse) { - this._abort(); - this.emit('fatalError', getText(MESSAGE.destRequestTimeout, this.opts.url)); - } + if (!this.hasResponse) + this._fatalError(MESSAGE.destRequestTimeout); } _onError (err) { @@ -113,7 +114,8 @@ export default class DestinationRequest extends EventEmitter { } else if (this._isDNSErr(err)) - this.emit('fatalError', getText(MESSAGE.cantResolveUrl, this.opts.url)); + this._fatalError(MESSAGE.cantResolveUrl); + else this.emit('error'); } diff --git a/test/client/fixtures/sandbox/code-instrumentation/getters-test.js b/test/client/fixtures/sandbox/code-instrumentation/getters-test.js index dd3ef88bf..9b3aa3838 100644 --- a/test/client/fixtures/sandbox/code-instrumentation/getters-test.js +++ b/test/client/fixtures/sandbox/code-instrumentation/getters-test.js @@ -169,6 +169,10 @@ test('document.baseURI (GH-920)', function () { } }; + documentMock.toString.toString = function () { + return 'function test() { [native code] }'; + }; + strictEqual(getProperty(documentMock, 'baseURI'), url); }); diff --git a/test/server/proxy-test.js b/test/server/proxy-test.js index 307843371..a5f845810 100644 --- a/test/server/proxy-test.js +++ b/test/server/proxy-test.js @@ -229,6 +229,12 @@ describe('Proxy', function () { res.end(); }); + app.get('/wait/:ms', function (req, res) { + setTimeout(function () { + res.end('text'); + }, req.params.ms); + }); + destServer = app.listen(2000); @@ -1415,5 +1421,41 @@ describe('Proxy', function () { }); }); + it('Should abort destination request after fatal error (GH-937)', function (done) { + var savedReqTimeout = DestinationRequest.TIMEOUT; + var fatalErrorEventCount = 0; + + DestinationRequest.TIMEOUT = 100; + + var destReq = new DestinationRequest({ + url: 'http://127.0.0.1:2000/wait/150', + protocol: 'http:', + hostname: '127.0.0.1', + host: '127.0.0.1:2000', + port: 2000, + path: '/wait/150', + method: 'GET', + body: new Buffer([]), + isXhr: false, + headers: [] + }); + + destReq.on('error', function () { + }); + destReq.on('fatalError', function () { + fatalErrorEventCount++; + }); + + setTimeout(function () { + destReq._onError({ message: 'ECONNREFUSED' }); + }, 50); + + setTimeout(function () { + DestinationRequest.TIMEOUT = savedReqTimeout; + + expect(fatalErrorEventCount).eql(1); + done(); + }, 150); + }); }); });