Stop piping child process output into logger only after close

web/packages/teleterm/src/mainProcess/mainProcess.ts

@@ -150,24 +150,15 @@ export default class MainProcess {
       },
     });
 
-    const tshdPassThroughLogger = createFileLoggerService({
+    this.logProcessExitAndError('tshd', this.tshdProcess);
+
+    createFileLoggerService({
       dev: this.settings.dev,
       dir: this.settings.userDataDir,
       name: 'tshd',
       loggerNameColor: LoggerColor.Cyan,
       passThroughMode: true,
-    });
-
-    tshdPassThroughLogger.pipeProcessOutputIntoLogger(this.tshdProcess.stdout);
-    tshdPassThroughLogger.pipeProcessOutputIntoLogger(this.tshdProcess.stderr);
-
-    this.tshdProcess.on('error', error => {
-      this.logger.error('tshd failed to start', error);
-    });
-
-    this.tshdProcess.once('exit', code => {
-      this.logger.info('tshd exited with code:', code);
-    });
+    }).pipeProcessOutputIntoLogger(this.tshdProcess);
   }
 
   private _initSharedProcess() {
@@ -178,28 +169,16 @@ export default class MainProcess {
         stdio: 'pipe', // stdio must be set to `pipe` as the gRPC server address is read from stdout
       }
     );
-    const sharedProcessPassThroughLogger = createFileLoggerService({
+
+    this.logProcessExitAndError('shared process', this.sharedProcess);
+
+    createFileLoggerService({
       dev: this.settings.dev,
       dir: this.settings.userDataDir,
       name: 'shared',
       loggerNameColor: LoggerColor.Yellow,
       passThroughMode: true,
-    });
-
-    sharedProcessPassThroughLogger.pipeProcessOutputIntoLogger(
-      this.sharedProcess.stdout
-    );
-    sharedProcessPassThroughLogger.pipeProcessOutputIntoLogger(
-      this.sharedProcess.stderr
-    );
-
-    this.sharedProcess.on('error', error => {
-      this.logger.error('shared process failed to start', error);
-    });
-
-    this.sharedProcess.once('exit', code => {
-      this.logger.info('shared process exited with code:', code);
-    });
+    }).pipeProcessOutputIntoLogger(this.sharedProcess);
   }
 
   private _initResolvingChildProcessAddresses(): void {
@@ -452,6 +431,27 @@ export default class MainProcess {
     daemonStop.stderr.setEncoding('utf-8');
     daemonStop.stderr.on('data', logger.error);
   }
+
+  private logProcessExitAndError(
+    processName: string,
+    childProcess: ChildProcess
+  ) {
+    childProcess.on('error', error => {
+      this.logger.error(`${processName} failed to start`, error);
+    });
+
+    childProcess.once('exit', (code, signal) => {
+      const codeOrSignal = [
+        // code can be 0, so we cannot just check it the same way as the signal.
+        code != null && `code ${code}`,
+        signal && `signal ${signal}`,
+      ]
+        .filter(Boolean)
+        .join(' ');
+
+      this.logger.info(`${processName} exited with ${codeOrSignal}`);
+    });
+  }
 }
 
 const DOCS_URL = 'https://goteleport.com/docs/use-teleport/teleport-connect/';

web/packages/teleterm/src/services/logger/loggerService.ts

@@ -25,6 +25,8 @@ import split2 from 'split2';
 
 import { Logger, LoggerService, NodeLoggerService } from './types';
 
+import type { ChildProcess } from 'node:child_process';
+
 /**
  * stdout logger should be used in child processes.
  * It sends logs directly to stdout, so the parent logger can process that output
@@ -104,10 +106,24 @@ export function createFileLoggerService(
   }
 
   return {
-    pipeProcessOutputIntoLogger(stream): void {
-      stream
-        .pipe(split2(line => ({ level: 'info', message: [line] })))
-        .pipe(instance);
+    pipeProcessOutputIntoLogger(childProcess: ChildProcess): void {
+      const splitStream = split2(line => ({ level: 'info', message: [line] }));
+
+      childProcess.stdout.pipe(splitStream, { end: false });
+      childProcess.stderr.pipe(splitStream, { end: false });
+
+      splitStream.pipe(instance);
+
+      // Because the .pipe calls above use { end: false }, the split stream won't end when the
+      // source streams end. This gives us a chance to wait for both stdout and stderr to get closed
+      // and only then end the stream.
+      //
+      // Otherwise the split stream would end the moment either stdout or stderr get closed, not
+      // giving a chance for the other stream to pipe its data to the instance stream. This could
+      // result in lost stderr logs when a process fails to start.
+      childProcess.on('close', () => {
+        splitStream.end();
+      });
     },
     createLogger(context = 'default'): Logger {
       const logger = instance.child({ context });

web/packages/teleterm/src/services/logger/types.ts

@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-import { Stream } from 'stream';
+import type { ChildProcess } from 'node:child_process';
 
 export interface Logger {
   error(...args: unknown[]): void;
@@ -27,5 +27,5 @@ export interface LoggerService {
 }
 
 export interface NodeLoggerService extends LoggerService {
-  pipeProcessOutputIntoLogger(stream: Stream): void;
+  pipeProcessOutputIntoLogger(childProcess: ChildProcess): void;
 }