fix `Server shutdown with "Error: Can't set headers after they are se…

…nt." ` (close DevExpress#937) (DevExpress#940)
AndreyBelym · Feb 28, 2019 · 54c5bd7 · 54c5bd7
1 parent 608782a
commit 54c5bd7
Show file tree

Hide file tree

Showing 3 changed files with 56 additions and 8 deletions.
diff --git 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
@@ -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
@@ -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);
+        });
     });
 });