diff --git a/web/packages/teleterm/src/mainProcess/mainProcess.ts b/web/packages/teleterm/src/mainProcess/mainProcess.ts index 4b919323eef14..dbcf905773a04 100644 --- a/web/packages/teleterm/src/mainProcess/mainProcess.ts +++ b/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/'; diff --git a/web/packages/teleterm/src/services/logger/loggerService.ts b/web/packages/teleterm/src/services/logger/loggerService.ts index ca4264b8309fe..2aff3444c068e 100644 --- a/web/packages/teleterm/src/services/logger/loggerService.ts +++ b/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 }); diff --git a/web/packages/teleterm/src/services/logger/types.ts b/web/packages/teleterm/src/services/logger/types.ts index 11bc49a4bc943..d185c6b4c0bd9 100644 --- a/web/packages/teleterm/src/services/logger/types.ts +++ b/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; }