fix(ECONRESET): fix it once and for all (#4258)

Author: pavelfeldman

src/server/pipeTransport.ts

@@ -15,15 +15,13 @@
  * limitations under the License.
  */
 
-import { helper, RegisteredListener } from './helper';
 import { ConnectionTransport, ProtocolRequest, ProtocolResponse } from './transport';
 import { makeWaitForNextTask } from '../utils/utils';
 import { debugLogger } from '../utils/debugLogger';
 
 export class PipeTransport implements ConnectionTransport {
   private _pipeWrite: NodeJS.WritableStream;
   private _pendingMessage = '';
-  private _eventListeners: RegisteredListener[];
   private _waitForNextTask = makeWaitForNextTask();
   private _closed = false;
 
@@ -32,17 +30,14 @@ export class PipeTransport implements ConnectionTransport {
 
   constructor(pipeWrite: NodeJS.WritableStream, pipeRead: NodeJS.ReadableStream) {
     this._pipeWrite = pipeWrite;
-    this._eventListeners = [
-      helper.addEventListener(pipeRead, 'data', buffer => this._dispatch(buffer)),
-      helper.addEventListener(pipeRead, 'close', () => {
-        this._closed = true;
-        helper.removeEventListeners(this._eventListeners);
-        if (this.onclose)
-          this.onclose.call(null);
-      }),
-      helper.addEventListener(pipeRead, 'error', e => debugLogger.log('error', e)),
-      helper.addEventListener(pipeWrite, 'error', e => debugLogger.log('error', e)),
-    ];
+    pipeRead.on('data', buffer => this._dispatch(buffer));
+    pipeRead.on('close', () => {
+      this._closed = true;
+      if (this.onclose)
+        this.onclose.call(null);
+    }),
+    pipeRead.on('error', e => debugLogger.log('error', e)),
+    pipeWrite.on('error', e => debugLogger.log('error', e)),
     this.onmessage = undefined;
     this.onclose = undefined;
   }

src/server/processLauncher.ts

@@ -85,6 +85,10 @@ export async function launchProcess(options: LaunchProcessOptions): Promise<Laun
         stdio,
       }
   );
+
+  // Prevent Unhandled 'error' event.
+  spawnedProcess.on('error', () => {});
+
   if (!spawnedProcess.pid) {
     let failed: (e: Error) => void;
     const failedPromise = new Promise<Error>((f, r) => failed = f);
@@ -197,6 +201,7 @@ export function waitForLine(progress: Progress, process: childProcess.ChildProce
       helper.addEventListener(rl, 'line', onLine),
       helper.addEventListener(rl, 'close', reject.bind(null, failError)),
       helper.addEventListener(process, 'exit', reject.bind(null, failError)),
+      // It is Ok to remove error handler because we did not create process and there is another listener.
       helper.addEventListener(process, 'error', reject.bind(null, failError))
     ];
 

src/server/transport.ts

@@ -99,7 +99,7 @@ export class WebSocketTransport implements ConnectionTransport {
       if (this.onclose)
         this.onclose.call(null);
     });
-    // Silently ignore all errors - we don't know what to do with them.
+    // Prevent Error: read ECONNRESET.
     this._ws.addEventListener('error', () => {});
   }
 

test/remoteServer.fixture.ts

@@ -74,7 +74,7 @@ export class RemoteServer {
       launchOptions,
       ...extraOptions,
     };
-    this._child = spawn('node', [path.join(__dirname, 'fixtures', 'closeme.js'), JSON.stringify(options)]);
+    this._child = spawn('node', [path.join(__dirname, 'fixtures', 'closeme.js'), JSON.stringify(options)], { env: process.env });
     this._child.on('error', (...args) => console.log('ERROR', ...args));
     this._exitPromise = new Promise(resolve => this._child.on('exit', (exitCode, signal) => {
       this._didExit = true;
@@ -94,6 +94,9 @@ export class RemoteServer {
         outputString = outputString.substring(match.index + match[0].length);
       }
     });
+    this._child.stderr.on('data', data => {
+      console.log(data.toString());
+    });
 
     this._wsEndpoint = await this.out('wsEndpoint');
   }