diff --git a/web/packages/teleterm/src/services/pty/fixtures/mocks.ts b/web/packages/teleterm/src/services/pty/fixtures/mocks.ts index 83ec46a66e58a..9a636df72b665 100644 --- a/web/packages/teleterm/src/services/pty/fixtures/mocks.ts +++ b/web/packages/teleterm/src/services/pty/fixtures/mocks.ts @@ -29,16 +29,30 @@ export class MockPtyProcess implements IPtyProcess { dispose() {} - onData() {} + onData() { + return () => {}; + } - onExit() {} + onExit() { + return () => {}; + } - onOpen() {} + onOpen() { + return () => {}; + } + + onStartError() { + return () => {}; + } getPid() { return 0; } + getPtyId() { + return '1234'; + } + async getCwd() { return ''; } diff --git a/web/packages/teleterm/src/services/pty/ptyHost/ptyEventsStreamHandler.ts b/web/packages/teleterm/src/services/pty/ptyHost/ptyEventsStreamHandler.ts index aa6f199217c89..04b08868bb041 100644 --- a/web/packages/teleterm/src/services/pty/ptyHost/ptyEventsStreamHandler.ts +++ b/web/packages/teleterm/src/services/pty/ptyHost/ptyEventsStreamHandler.ts @@ -16,6 +16,8 @@ import { ClientDuplexStream } from '@grpc/grpc-js'; +import Logger from 'teleterm/logger'; + import { PtyClientEvent, PtyEventData, @@ -25,15 +27,22 @@ import { } from 'teleterm/sharedProcess/ptyHost'; export class PtyEventsStreamHandler { + private logger: Logger; + constructor( - private readonly stream: ClientDuplexStream - ) {} + private readonly stream: ClientDuplexStream, + ptyId: string + ) { + this.logger = new Logger(`PtyEventsStreamHandler ${ptyId}`); + } /** * Client -> Server stream events */ start(columns: number, rows: number): void { + this.logger.info('Start'); + this.writeOrThrow( new PtyClientEvent().setStart( new PtyEventStart().setColumns(columns).setRows(rows) @@ -56,6 +65,8 @@ export class PtyEventsStreamHandler { } dispose(): void { + this.logger.info('Dispose'); + this.stream.end(); this.stream.removeAllListeners(); } @@ -64,30 +75,51 @@ export class PtyEventsStreamHandler { * Stream -> Client stream events */ - onOpen(callback: () => void): void { - this.stream.addListener('data', (event: PtyServerEvent) => { - if (event.hasOpen()) { - callback(); + onOpen(callback: () => void): RemoveListenerFunction { + return this.addDataListenerAndReturnRemovalFunction( + (event: PtyServerEvent) => { + if (event.hasOpen()) { + callback(); + } } - }); + ); } - onData(callback: (data: string) => void): void { - this.stream.addListener('data', (event: PtyServerEvent) => { - if (event.hasData()) { - callback(event.getData().getMessage()); + onData(callback: (data: string) => void): RemoveListenerFunction { + return this.addDataListenerAndReturnRemovalFunction( + (event: PtyServerEvent) => { + if (event.hasData()) { + callback(event.getData().getMessage()); + } } - }); + ); } onExit( callback: (reason: { exitCode: number; signal?: number }) => void - ): void { - this.stream.addListener('data', (event: PtyServerEvent) => { - if (event.hasExit()) { - callback(event.getExit().toObject()); + ): RemoveListenerFunction { + return this.addDataListenerAndReturnRemovalFunction( + (event: PtyServerEvent) => { + if (event.hasExit()) { + this.logger.info('On exit', event.getExit().toObject()); + callback(event.getExit().toObject()); + } } - }); + ); + } + + onStartError(callback: (message: string) => void): RemoveListenerFunction { + return this.addDataListenerAndReturnRemovalFunction( + (event: PtyServerEvent) => { + if (event.hasStartError()) { + this.logger.info( + 'On start error', + event.getStartError().toObject().message + ); + callback(event.getStartError().toObject().message); + } + } + ); } private writeOrThrow(event: PtyClientEvent) { @@ -97,4 +129,16 @@ export class PtyEventsStreamHandler { } }); } + + private addDataListenerAndReturnRemovalFunction( + callback: (event: PtyServerEvent) => void + ) { + this.stream.addListener('data', callback); + + return () => { + this.stream.removeListener('data', callback); + }; + } } + +type RemoveListenerFunction = () => void; diff --git a/web/packages/teleterm/src/services/pty/ptyHost/ptyHostClient.ts b/web/packages/teleterm/src/services/pty/ptyHost/ptyHostClient.ts index 053e34a7cd725..80200a67bc5ae 100644 --- a/web/packages/teleterm/src/services/pty/ptyHost/ptyHostClient.ts +++ b/web/packages/teleterm/src/services/pty/ptyHost/ptyHostClient.ts @@ -72,7 +72,7 @@ export function createPtyHostClient( const metadata = new Metadata(); metadata.set('ptyId', ptyId); const stream = client.exchangeEvents(metadata); - return new PtyEventsStreamHandler(stream); + return new PtyEventsStreamHandler(stream, ptyId); }, }; } diff --git a/web/packages/teleterm/src/services/pty/ptyHost/ptyProcess.ts b/web/packages/teleterm/src/services/pty/ptyHost/ptyProcess.ts index 908d2ea5f9046..353b3baf05eaf 100644 --- a/web/packages/teleterm/src/services/pty/ptyHost/ptyProcess.ts +++ b/web/packages/teleterm/src/services/pty/ptyHost/ptyProcess.ts @@ -25,6 +25,10 @@ export function createPtyProcess( const exchangeEventsStream = ptyHostClient.exchangeEvents(ptyId); return { + getPtyId() { + return ptyId; + }, + /** * Client -> Server stream events */ @@ -49,18 +53,20 @@ export function createPtyProcess( * Server -> Client stream events */ - onData(callback: (data: string) => void): void { - exchangeEventsStream.onData(callback); + onData(callback: (data: string) => void) { + return exchangeEventsStream.onData(callback); + }, + + onOpen(callback: () => void) { + return exchangeEventsStream.onOpen(callback); }, - onOpen(callback: () => void): void { - exchangeEventsStream.onOpen(callback); + onExit(callback: (reason: { exitCode: number; signal?: number }) => void) { + return exchangeEventsStream.onExit(callback); }, - onExit( - callback: (reason: { exitCode: number; signal?: number }) => void - ): void { - exchangeEventsStream.onExit(callback); + onStartError(callback: (message: string) => void) { + return exchangeEventsStream.onStartError(callback); }, /** diff --git a/web/packages/teleterm/src/sharedProcess/api/proto/ptyHostService.proto b/web/packages/teleterm/src/sharedProcess/api/proto/ptyHostService.proto index d433f7a2e7145..c52201800ed88 100644 --- a/web/packages/teleterm/src/sharedProcess/api/proto/ptyHostService.proto +++ b/web/packages/teleterm/src/sharedProcess/api/proto/ptyHostService.proto @@ -51,6 +51,7 @@ message PtyServerEvent { PtyEventData data = 2; PtyEventOpen open = 3; PtyEventExit exit = 4; + PtyEventStartError start_error = 5; } } @@ -75,6 +76,10 @@ message PtyEventExit { optional uint32 signal = 2; } +message PtyEventStartError { + string message = 1; +} + message PtyCwd { string cwd = 1; } diff --git a/web/packages/teleterm/src/sharedProcess/api/protogen/ptyHostService_grpc_pb.js b/web/packages/teleterm/src/sharedProcess/api/protogen/ptyHostService_grpc_pb.js index 8465390549c68..a81f3650c83f8 100644 --- a/web/packages/teleterm/src/sharedProcess/api/protogen/ptyHostService_grpc_pb.js +++ b/web/packages/teleterm/src/sharedProcess/api/protogen/ptyHostService_grpc_pb.js @@ -1,5 +1,23 @@ // GENERATED CODE -- DO NOT EDIT! +// Original file comments: +// Copyright 2023 Gravitational, Inc +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// TODO(ravicious): Before introducing any changes, move this file to the /proto dir and +// remove the generate-grpc-shared script. +// 'use strict'; var grpc = require('@grpc/grpc-js'); var ptyHostService_pb = require('./ptyHostService_pb.js'); diff --git a/web/packages/teleterm/src/sharedProcess/api/protogen/ptyHostService_pb.d.ts b/web/packages/teleterm/src/sharedProcess/api/protogen/ptyHostService_pb.d.ts index af0faf4ee0f39..31ab1d6bfe3cd 100644 --- a/web/packages/teleterm/src/sharedProcess/api/protogen/ptyHostService_pb.d.ts +++ b/web/packages/teleterm/src/sharedProcess/api/protogen/ptyHostService_pb.d.ts @@ -150,6 +150,11 @@ export class PtyServerEvent extends jspb.Message { getExit(): PtyEventExit | undefined; setExit(value?: PtyEventExit): PtyServerEvent; + hasStartError(): boolean; + clearStartError(): void; + getStartError(): PtyEventStartError | undefined; + setStartError(value?: PtyEventStartError): PtyServerEvent; + getEventCase(): PtyServerEvent.EventCase; serializeBinary(): Uint8Array; @@ -168,6 +173,7 @@ export namespace PtyServerEvent { data?: PtyEventData.AsObject, open?: PtyEventOpen.AsObject, exit?: PtyEventExit.AsObject, + startError?: PtyEventStartError.AsObject, } export enum EventCase { @@ -176,6 +182,7 @@ export namespace PtyServerEvent { DATA = 2, OPEN = 3, EXIT = 4, + START_ERROR = 5, } } @@ -289,6 +296,26 @@ export namespace PtyEventExit { } } +export class PtyEventStartError extends jspb.Message { + getMessage(): string; + setMessage(value: string): PtyEventStartError; + + serializeBinary(): Uint8Array; + toObject(includeInstance?: boolean): PtyEventStartError.AsObject; + static toObject(includeInstance: boolean, msg: PtyEventStartError): PtyEventStartError.AsObject; + static extensions: {[key: number]: jspb.ExtensionFieldInfo}; + static extensionsBinary: {[key: number]: jspb.ExtensionFieldBinaryInfo}; + static serializeBinaryToWriter(message: PtyEventStartError, writer: jspb.BinaryWriter): void; + static deserializeBinary(bytes: Uint8Array): PtyEventStartError; + static deserializeBinaryFromReader(message: PtyEventStartError, reader: jspb.BinaryReader): PtyEventStartError; +} + +export namespace PtyEventStartError { + export type AsObject = { + message: string, + } +} + export class PtyCwd extends jspb.Message { getCwd(): string; setCwd(value: string): PtyCwd; diff --git a/web/packages/teleterm/src/sharedProcess/api/protogen/ptyHostService_pb.js b/web/packages/teleterm/src/sharedProcess/api/protogen/ptyHostService_pb.js index 390ba08ad8edd..a70320eb79331 100644 --- a/web/packages/teleterm/src/sharedProcess/api/protogen/ptyHostService_pb.js +++ b/web/packages/teleterm/src/sharedProcess/api/protogen/ptyHostService_pb.js @@ -13,7 +13,13 @@ var jspb = require('google-protobuf'); var goog = jspb; -var global = Function('return this')(); +var global = (function() { + if (this) { return this; } + if (typeof window !== 'undefined') { return window; } + if (typeof global !== 'undefined') { return global; } + if (typeof self !== 'undefined') { return self; } + return Function('return this')(); +}.call(null)); var google_protobuf_struct_pb = require('google-protobuf/google/protobuf/struct_pb.js'); goog.object.extend(proto, google_protobuf_struct_pb); @@ -26,6 +32,7 @@ goog.exportSymbol('proto.PtyEventExit', null, global); goog.exportSymbol('proto.PtyEventOpen', null, global); goog.exportSymbol('proto.PtyEventResize', null, global); goog.exportSymbol('proto.PtyEventStart', null, global); +goog.exportSymbol('proto.PtyEventStartError', null, global); goog.exportSymbol('proto.PtyId', null, global); goog.exportSymbol('proto.PtyServerEvent', null, global); goog.exportSymbol('proto.PtyServerEvent.EventCase', null, global); @@ -218,6 +225,27 @@ if (goog.DEBUG && !COMPILED) { */ proto.PtyEventExit.displayName = 'proto.PtyEventExit'; } +/** + * Generated by JsPbCodeGenerator. + * @param {Array=} opt_data Optional initial data array, typically from a + * server response, or constructed directly in Javascript. The array is used + * in place and becomes part of the constructed object. It is not cloned. + * If no data is provided, the constructed object will be empty, but still + * valid. + * @extends {jspb.Message} + * @constructor + */ +proto.PtyEventStartError = function(opt_data) { + jspb.Message.initialize(this, opt_data, 0, -1, null, null); +}; +goog.inherits(proto.PtyEventStartError, jspb.Message); +if (goog.DEBUG && !COMPILED) { + /** + * @public + * @override + */ + proto.PtyEventStartError.displayName = 'proto.PtyEventStartError'; +} /** * Generated by JsPbCodeGenerator. * @param {Array=} opt_data Optional initial data array, typically from a @@ -973,7 +1001,7 @@ proto.PtyClientEvent.prototype.hasData = function() { * @private {!Array>} * @const */ -proto.PtyServerEvent.oneofGroups_ = [[1,2,3,4]]; +proto.PtyServerEvent.oneofGroups_ = [[1,2,3,4,5]]; /** * @enum {number} @@ -983,7 +1011,8 @@ proto.PtyServerEvent.EventCase = { RESIZE: 1, DATA: 2, OPEN: 3, - EXIT: 4 + EXIT: 4, + START_ERROR: 5 }; /** @@ -1027,7 +1056,8 @@ proto.PtyServerEvent.toObject = function(includeInstance, msg) { resize: (f = msg.getResize()) && proto.PtyEventResize.toObject(includeInstance, f), data: (f = msg.getData()) && proto.PtyEventData.toObject(includeInstance, f), open: (f = msg.getOpen()) && proto.PtyEventOpen.toObject(includeInstance, f), - exit: (f = msg.getExit()) && proto.PtyEventExit.toObject(includeInstance, f) + exit: (f = msg.getExit()) && proto.PtyEventExit.toObject(includeInstance, f), + startError: (f = msg.getStartError()) && proto.PtyEventStartError.toObject(includeInstance, f) }; if (includeInstance) { @@ -1084,6 +1114,11 @@ proto.PtyServerEvent.deserializeBinaryFromReader = function(msg, reader) { reader.readMessage(value,proto.PtyEventExit.deserializeBinaryFromReader); msg.setExit(value); break; + case 5: + var value = new proto.PtyEventStartError; + reader.readMessage(value,proto.PtyEventStartError.deserializeBinaryFromReader); + msg.setStartError(value); + break; default: reader.skipField(); break; @@ -1145,6 +1180,14 @@ proto.PtyServerEvent.serializeBinaryToWriter = function(message, writer) { proto.PtyEventExit.serializeBinaryToWriter ); } + f = message.getStartError(); + if (f != null) { + writer.writeMessage( + 5, + f, + proto.PtyEventStartError.serializeBinaryToWriter + ); + } }; @@ -1296,6 +1339,43 @@ proto.PtyServerEvent.prototype.hasExit = function() { }; +/** + * optional PtyEventStartError start_error = 5; + * @return {?proto.PtyEventStartError} + */ +proto.PtyServerEvent.prototype.getStartError = function() { + return /** @type{?proto.PtyEventStartError} */ ( + jspb.Message.getWrapperField(this, proto.PtyEventStartError, 5)); +}; + + +/** + * @param {?proto.PtyEventStartError|undefined} value + * @return {!proto.PtyServerEvent} returns this +*/ +proto.PtyServerEvent.prototype.setStartError = function(value) { + return jspb.Message.setOneofWrapperField(this, 5, proto.PtyServerEvent.oneofGroups_[0], value); +}; + + +/** + * Clears the message field making it undefined. + * @return {!proto.PtyServerEvent} returns this + */ +proto.PtyServerEvent.prototype.clearStartError = function() { + return this.setStartError(undefined); +}; + + +/** + * Returns whether this field is set. + * @return {boolean} + */ +proto.PtyServerEvent.prototype.hasStartError = function() { + return jspb.Message.getField(this, 5) != null; +}; + + @@ -2028,6 +2108,136 @@ proto.PtyEventExit.prototype.hasSignal = function() { +if (jspb.Message.GENERATE_TO_OBJECT) { +/** + * Creates an object representation of this proto. + * Field names that are reserved in JavaScript and will be renamed to pb_name. + * Optional fields that are not set will be set to undefined. + * To access a reserved field use, foo.pb_, eg, foo.pb_default. + * For the list of reserved names please see: + * net/proto2/compiler/js/internal/generator.cc#kKeyword. + * @param {boolean=} opt_includeInstance Deprecated. whether to include the + * JSPB instance for transitional soy proto support: + * http://goto/soy-param-migration + * @return {!Object} + */ +proto.PtyEventStartError.prototype.toObject = function(opt_includeInstance) { + return proto.PtyEventStartError.toObject(opt_includeInstance, this); +}; + + +/** + * Static version of the {@see toObject} method. + * @param {boolean|undefined} includeInstance Deprecated. Whether to include + * the JSPB instance for transitional soy proto support: + * http://goto/soy-param-migration + * @param {!proto.PtyEventStartError} msg The msg instance to transform. + * @return {!Object} + * @suppress {unusedLocalVariables} f is only used for nested messages + */ +proto.PtyEventStartError.toObject = function(includeInstance, msg) { + var f, obj = { + message: jspb.Message.getFieldWithDefault(msg, 1, "") + }; + + if (includeInstance) { + obj.$jspbMessageInstance = msg; + } + return obj; +}; +} + + +/** + * Deserializes binary data (in protobuf wire format). + * @param {jspb.ByteSource} bytes The bytes to deserialize. + * @return {!proto.PtyEventStartError} + */ +proto.PtyEventStartError.deserializeBinary = function(bytes) { + var reader = new jspb.BinaryReader(bytes); + var msg = new proto.PtyEventStartError; + return proto.PtyEventStartError.deserializeBinaryFromReader(msg, reader); +}; + + +/** + * Deserializes binary data (in protobuf wire format) from the + * given reader into the given message object. + * @param {!proto.PtyEventStartError} msg The message object to deserialize into. + * @param {!jspb.BinaryReader} reader The BinaryReader to use. + * @return {!proto.PtyEventStartError} + */ +proto.PtyEventStartError.deserializeBinaryFromReader = function(msg, reader) { + while (reader.nextField()) { + if (reader.isEndGroup()) { + break; + } + var field = reader.getFieldNumber(); + switch (field) { + case 1: + var value = /** @type {string} */ (reader.readString()); + msg.setMessage(value); + break; + default: + reader.skipField(); + break; + } + } + return msg; +}; + + +/** + * Serializes the message to binary data (in protobuf wire format). + * @return {!Uint8Array} + */ +proto.PtyEventStartError.prototype.serializeBinary = function() { + var writer = new jspb.BinaryWriter(); + proto.PtyEventStartError.serializeBinaryToWriter(this, writer); + return writer.getResultBuffer(); +}; + + +/** + * Serializes the given message to binary data (in protobuf wire + * format), writing to the given BinaryWriter. + * @param {!proto.PtyEventStartError} message + * @param {!jspb.BinaryWriter} writer + * @suppress {unusedLocalVariables} f is only used for nested messages + */ +proto.PtyEventStartError.serializeBinaryToWriter = function(message, writer) { + var f = undefined; + f = message.getMessage(); + if (f.length > 0) { + writer.writeString( + 1, + f + ); + } +}; + + +/** + * optional string message = 1; + * @return {string} + */ +proto.PtyEventStartError.prototype.getMessage = function() { + return /** @type {string} */ (jspb.Message.getFieldWithDefault(this, 1, "")); +}; + + +/** + * @param {string} value + * @return {!proto.PtyEventStartError} returns this + */ +proto.PtyEventStartError.prototype.setMessage = function(value) { + return jspb.Message.setProto3StringField(this, 1, value); +}; + + + + + if (jspb.Message.GENERATE_TO_OBJECT) { /** * Creates an object representation of this proto. diff --git a/web/packages/teleterm/src/sharedProcess/ptyHost/ptyEventsStreamHandler.ts b/web/packages/teleterm/src/sharedProcess/ptyHost/ptyEventsStreamHandler.ts index 97a6c655fe570..3b500aef797ee 100644 --- a/web/packages/teleterm/src/sharedProcess/ptyHost/ptyEventsStreamHandler.ts +++ b/web/packages/teleterm/src/sharedProcess/ptyHost/ptyEventsStreamHandler.ts @@ -25,6 +25,7 @@ import { PtyEventOpen, PtyEventResize, PtyEventStart, + PtyEventStartError, PtyServerEvent, } from '../api/protogen/ptyHostService_pb'; @@ -75,6 +76,13 @@ export class PtyEventsStreamHandler { ) ) ); + this.ptyProcess.onStartError(message => { + this.stream.write( + new PtyServerEvent().setStartError( + new PtyEventStartError().setMessage(message) + ) + ); + }); this.ptyProcess.start(event.getColumns(), event.getRows()); this.logger.info(`stream has started`); } diff --git a/web/packages/teleterm/src/sharedProcess/ptyHost/ptyProcess.ts b/web/packages/teleterm/src/sharedProcess/ptyHost/ptyProcess.ts index e933c0be9af14..a6356cde03685 100644 --- a/web/packages/teleterm/src/sharedProcess/ptyHost/ptyProcess.ts +++ b/web/packages/teleterm/src/sharedProcess/ptyHost/ptyProcess.ts @@ -48,23 +48,33 @@ export class PtyProcess extends EventEmitter implements IPtyProcess { ); } + getPtyId() { + return this.options.ptyId; + } + start(cols: number, rows: number) { - // TODO(ravicious): Set argv0 when node-pty adds support for it. - // https://github.com/microsoft/node-pty/issues/472 - this._process = nodePTY.spawn(this.options.path, this.options.args, { - cols, - rows, - name: 'xterm-color', - // HOME should be always defined. But just in case it isn't let's use the cwd from process. - // https://unix.stackexchange.com/questions/123858 - cwd: this.options.cwd || getDefaultCwd(this.options.env), - env: this.options.env, - // Turn off ConPTY due to an uncaught exception being thrown when a PTY is closed. - useConpty: false, - }); + try { + // TODO(ravicious): Set argv0 when node-pty adds support for it. + // https://github.com/microsoft/node-pty/issues/472 + this._process = nodePTY.spawn(this.options.path, this.options.args, { + cols, + rows, + name: 'xterm-color', + // HOME should be always defined. But just in case it isn't let's use the cwd from process. + // https://unix.stackexchange.com/questions/123858 + cwd: this.options.cwd || getDefaultCwd(this.options.env), + env: this.options.env, + // Turn off ConPTY due to an uncaught exception being thrown when a PTY is closed. + useConpty: false, + }); + } catch (error) { + this._logger.error(error); + this.handleStartError(error); + return; + } this._setStatus('open'); - this.emit(TermEventEnum.OPEN); + this.emit(TermEventEnum.Open); this._process.onData(data => this._handleData(data)); this._process.onExit(ev => this._handleExit(ev)); @@ -114,15 +124,35 @@ export class PtyProcess extends EventEmitter implements IPtyProcess { } onData(cb: (data: string) => void) { - this.addListener(TermEventEnum.DATA, cb); + return this.addListenerAndReturnRemovalFunction(TermEventEnum.Data, cb); } onOpen(cb: () => void) { - this.addListener(TermEventEnum.OPEN, cb); + return this.addListenerAndReturnRemovalFunction(TermEventEnum.Open, cb); } onExit(cb: (ev: { exitCode: number; signal?: number }) => void) { - this.addListener(TermEventEnum.EXIT, cb); + return this.addListenerAndReturnRemovalFunction(TermEventEnum.Exit, cb); + } + + onStartError(cb: (message: string) => void) { + return this.addListenerAndReturnRemovalFunction( + TermEventEnum.StartError, + cb + ); + } + + private addListenerAndReturnRemovalFunction( + eventName: TermEventEnum, + listener: (...args: any[]) => void + ) { + this.addListener(eventName, listener); + + // The removal function is not used from within the shared process code, it is returned only to + // comply with the IPtyProcess interface. + return () => { + this.removeListener(eventName, listener); + }; } private getPid() { @@ -130,7 +160,7 @@ export class PtyProcess extends EventEmitter implements IPtyProcess { } private _flushBuffer() { - this.emit(TermEventEnum.DATA, this._attachedBuffer); + this.emit(TermEventEnum.Data, this._attachedBuffer); this._attachedBuffer = null; clearTimeout(this._attachedBufferTimer); this._attachedBufferTimer = null; @@ -146,7 +176,7 @@ export class PtyProcess extends EventEmitter implements IPtyProcess { } private _handleExit(e: { exitCode: number; signal?: number }) { - this.emit(TermEventEnum.EXIT, e); + this.emit(TermEventEnum.Exit, e); this._logger.info(`pty has been terminated with exit code: ${e.exitCode}`); this._setStatus('terminated'); } @@ -156,26 +186,35 @@ export class PtyProcess extends EventEmitter implements IPtyProcess { if (this._buffered) { this._pushToBuffer(data); } else { - this.emit(TermEventEnum.DATA, data); + this.emit(TermEventEnum.Data, data); } } catch (err) { this._logger.error('failed to parse incoming message.', err); } } + private handleStartError(error: Error) { + const command = `${this.options.path} ${this.options.args.join(' ')}`; + this.emit( + TermEventEnum.StartError, + `Cannot execute ${command}: ${error.message}` + ); + } + private _setStatus(value: Status) { this._status = value; this._logger.info(`status -> ${value}`); } } -export const TermEventEnum = { - CLOSE: 'terminal.close', - RESET: 'terminal.reset', - DATA: 'terminal.data', - OPEN: 'terminal.open', - EXIT: 'terminal.exit', -}; +export enum TermEventEnum { + Close = 'terminal.close', + Reset = 'terminal.reset', + Data = 'terminal.data', + Open = 'terminal.open', + Exit = 'terminal.exit', + StartError = 'terminal.start_error', +} async function getWorkingDirectory(pid: number): Promise { switch (process.platform) { diff --git a/web/packages/teleterm/src/sharedProcess/ptyHost/types.ts b/web/packages/teleterm/src/sharedProcess/ptyHost/types.ts index e9c1d0e60a214..32edc963cd124 100644 --- a/web/packages/teleterm/src/sharedProcess/ptyHost/types.ts +++ b/web/packages/teleterm/src/sharedProcess/ptyHost/types.ts @@ -23,12 +23,22 @@ export type PtyProcessOptions = { }; export type IPtyProcess = { + start(cols: number, rows: number): void; write(data: string): void; resize(cols: number, rows: number): void; dispose(): void; - onData(cb: (data: string) => void): void; - onOpen(cb: () => void): void; - start(cols: number, rows: number): void; - onExit(cb: (ev: { exitCode: number; signal?: number }) => void): void; getCwd(): Promise; + getPtyId(): string; + // The listener removal functions are used only on the frontend app side from the renderer process. + // They're not used in the shared process. However, IPtyProcess is a type shared between both, so + // both sides need to return them. In the future we should consider defining two separate types + // for both cases. + onData(cb: (data: string) => void): RemoveListenerFunction; + onOpen(cb: () => void): RemoveListenerFunction; + onStartError(cb: (message: string) => void): RemoveListenerFunction; + onExit( + cb: (ev: { exitCode: number; signal?: number }) => void + ): RemoveListenerFunction; }; + +type RemoveListenerFunction = () => void; diff --git a/web/packages/teleterm/src/ui/DocumentTerminal/DocumentTerminal.tsx b/web/packages/teleterm/src/ui/DocumentTerminal/DocumentTerminal.tsx index e594e6cdc2e35..27c7de61232c5 100644 --- a/web/packages/teleterm/src/ui/DocumentTerminal/DocumentTerminal.tsx +++ b/web/packages/teleterm/src/ui/DocumentTerminal/DocumentTerminal.tsx @@ -15,21 +15,18 @@ limitations under the License. */ import React from 'react'; -import { Flex, Text, ButtonPrimary } from 'design'; -import { Danger } from 'design/Alert'; import { FileTransferActionBar, FileTransfer, FileTransferContextProvider, } from 'shared/components/FileTransfer'; -import { Attempt } from 'shared/hooks/useAsync'; import Document from 'teleterm/ui/Document'; import { useAppContext } from 'teleterm/ui/appContextProvider'; -import { assertUnreachable } from 'teleterm/ui/utils'; import { isDocumentTshNodeWithServerId } from 'teleterm/ui/services/workspacesService'; import { Terminal } from './Terminal'; +import { Reconnect } from './Reconnect'; import { useDocumentTerminal } from './useDocumentTerminal'; import { useTshFileTransferHandlers } from './useTshFileTransferHandlers'; @@ -42,27 +39,27 @@ export function DocumentTerminal(props: { const ctx = useAppContext(); const { configService } = ctx.mainProcessClient; const { visible, doc } = props; - const { attempt, reconnect } = useDocumentTerminal(doc); - const ptyProcess = attempt.data?.ptyProcess; + const { attempt, initializePtyProcess } = useDocumentTerminal(doc); const { upload, download } = useTshFileTransferHandlers(); const unsanitizedTerminalFontFamily = configService.get( 'terminal.fontFamily' ).value; const terminalFontSize = configService.get('terminal.fontSize').value; - // Creating a new terminal might fail for multiple reasons, for example: + // Initializing a new terminal might fail for multiple reasons, for example: // // * The user tried to execute `tsh ssh user@host` from the command bar and the request which // tries to resolve `host` to a server object failed due to a network or cluster error. // * The PTY service has failed to create a new PTY process. if (attempt.status === 'error') { return ( - + + + ); } @@ -120,9 +117,16 @@ export function DocumentTerminal(props: { autoFocusDisabled={true} > {$fileTransfer} - {ptyProcess && ( + {attempt.status === 'success' && ( and re-run all hooks for the new PTY process. + key={attempt.data.ptyProcess.getPtyId()} + docKind={doc.kind} + ptyProcess={attempt.data.ptyProcess} + reconnect={initializePtyProcess} visible={props.visible} unsanitizedFontFamily={unsanitizedTerminalFontFamily} fontSize={terminalFontSize} @@ -132,55 +136,3 @@ export function DocumentTerminal(props: { ); } - -function DocumentReconnect(props: { - visible: boolean; - doc: types.DocumentTerminal; - attempt: Attempt; - reconnect: () => void; -}) { - const { message, buttonText } = getReconnectCopy(props.doc); - - return ( - - - - {message} - - - {props.attempt.statusText} - - {buttonText} - - - - - ); -} - -function getReconnectCopy(doc: types.DocumentTerminal) { - switch (doc.kind) { - case 'doc.terminal_tsh_node': { - return { - message: 'This SSH connection is currently offline.', - buttonText: 'Reconnect', - }; - } - case 'doc.gateway_cli_client': - case 'doc.terminal_shell': - case 'doc.terminal_tsh_kube': { - return { - message: 'Ran into an error when starting the terminal session.', - buttonText: 'Retry', - }; - } - default: - assertUnreachable(doc); - } -} diff --git a/web/packages/teleterm/src/ui/DocumentTerminal/Reconnect.tsx b/web/packages/teleterm/src/ui/DocumentTerminal/Reconnect.tsx new file mode 100644 index 0000000000000..eb313b5a493df --- /dev/null +++ b/web/packages/teleterm/src/ui/DocumentTerminal/Reconnect.tsx @@ -0,0 +1,84 @@ +/** + * Copyright 2023 Gravitational, Inc + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import React from 'react'; +import { Flex, Text, ButtonPrimary } from 'design'; +import { Danger } from 'design/Alert'; +import { Attempt } from 'shared/hooks/useAsync'; + +import { assertUnreachable } from 'teleterm/ui/utils'; + +import type * as types from 'teleterm/ui/services/workspacesService'; + +export function Reconnect(props: { + docKind: types.DocumentTerminal['kind']; + attempt: Attempt; + reconnect: () => void; +}) { + const { message, buttonText } = getReconnectCopy(props.docKind); + + return ( + + + {message} + + + + + {props.attempt.statusText} + + + + {buttonText} + + + + ); +} + +function getReconnectCopy(docKind: types.DocumentTerminal['kind']) { + switch (docKind) { + case 'doc.terminal_tsh_node': { + return { + message: 'This SSH connection is currently offline.', + buttonText: 'Reconnect', + }; + } + case 'doc.gateway_cli_client': + case 'doc.terminal_shell': + case 'doc.terminal_tsh_kube': { + return { + message: 'Ran into an error when starting the terminal session.', + buttonText: 'Retry', + }; + } + default: + assertUnreachable(docKind); + } +} diff --git a/web/packages/teleterm/src/ui/DocumentTerminal/Terminal/Terminal.tsx b/web/packages/teleterm/src/ui/DocumentTerminal/Terminal/Terminal.tsx index 38eb2c415834f..29353a7bf7a02 100644 --- a/web/packages/teleterm/src/ui/DocumentTerminal/Terminal/Terminal.tsx +++ b/web/packages/teleterm/src/ui/DocumentTerminal/Terminal/Terminal.tsx @@ -14,17 +14,28 @@ See the License for the specific language governing permissions and limitations under the License. */ -import React, { useEffect, useRef } from 'react'; +import React, { useEffect, useRef, useState } from 'react'; import styled from 'styled-components'; import { Box, Flex } from 'design'; import { debounce } from 'shared/utils/highbar'; +import { + Attempt, + makeEmptyAttempt, + makeErrorAttempt, + makeSuccessAttempt, +} from 'shared/hooks/useAsync'; import { IPtyProcess } from 'teleterm/sharedProcess/ptyHost'; +import { DocumentTerminal } from 'teleterm/ui/services/workspacesService'; + +import { Reconnect } from '../Reconnect'; import XTermCtrl from './ctrl'; type TerminalProps = { + docKind: DocumentTerminal['kind']; ptyProcess: IPtyProcess; + reconnect: () => void; visible: boolean; /** * This value can be provided by the user and is unsanitized. This means that it cannot be directly interpolated @@ -40,13 +51,27 @@ type TerminalProps = { export function Terminal(props: TerminalProps) { const refElement = useRef(); const refCtrl = useRef(); + const [startPtyProcessAttempt, setStartPtyProcessAttempt] = useState< + Attempt + >(makeEmptyAttempt()); useEffect(() => { + const removeOnStartErrorListener = props.ptyProcess.onStartError( + message => { + setStartPtyProcessAttempt(makeErrorAttempt(message)); + } + ); + + const removeOnOpenListener = props.ptyProcess.onOpen(() => { + setStartPtyProcessAttempt(makeSuccessAttempt(undefined)); + }); + const ctrl = new XTermCtrl(props.ptyProcess, { el: refElement.current, fontSize: props.fontSize, }); + // Start the PTY process. ctrl.open(); ctrl.term.onKey(event => { @@ -62,6 +87,8 @@ export function Terminal(props: TerminalProps) { }, 100); return () => { + removeOnStartErrorListener(); + removeOnOpenListener(); handleEnterPress.cancel(); ctrl.destroy(); }; @@ -83,9 +110,20 @@ export function Terminal(props: TerminalProps) { width="100%" style={{ overflow: 'hidden' }} > + {startPtyProcessAttempt.status === 'error' && ( + + )} ); diff --git a/web/packages/teleterm/src/ui/DocumentTerminal/Terminal/ctrl.ts b/web/packages/teleterm/src/ui/DocumentTerminal/Terminal/ctrl.ts index 1564514d1a770..aa72568507bd3 100644 --- a/web/packages/teleterm/src/ui/DocumentTerminal/Terminal/ctrl.ts +++ b/web/packages/teleterm/src/ui/DocumentTerminal/Terminal/ctrl.ts @@ -37,6 +37,7 @@ export default class TtyTerminal { private resizeHandler: IDisposable; private debouncedResize: () => void; private logger = new Logger('lib/term/terminal'); + private removePtyProcessOnDataListener: () => void; constructor(private ptyProcess: IPtyProcess, private options: Options) { this.el = options.el; @@ -84,8 +85,14 @@ export default class TtyTerminal { this.ptyProcess.resize(size.cols, size.rows); }); - this.ptyProcess.onData(data => this.handleData(data)); + this.removePtyProcessOnDataListener = this.ptyProcess.onData(data => + this.handleData(data) + ); + // TODO(ravicious): Don't call start if the process was already started. + // This is what is causing the terminal to visually repeat the input on hot reload. + // The shared process version of PtyProcess knows whether it was started or not (the status + // field), so it's a matter of exposing this field through gRPC and reading it here. this.ptyProcess.start(this.term.cols, this.term.rows); window.addEventListener('resize', this.debouncedResize); @@ -105,7 +112,7 @@ export default class TtyTerminal { } destroy(): void { - this.ptyProcess?.dispose(); + this.removePtyProcessOnDataListener?.(); this.term?.dispose(); this.fitAddon.dispose(); this.resizeHandler?.dispose(); diff --git a/web/packages/teleterm/src/ui/DocumentTerminal/useDocumentTerminal.test.tsx b/web/packages/teleterm/src/ui/DocumentTerminal/useDocumentTerminal.test.tsx index 65827de2c4dc8..991c54285b1a8 100644 --- a/web/packages/teleterm/src/ui/DocumentTerminal/useDocumentTerminal.test.tsx +++ b/web/packages/teleterm/src/ui/DocumentTerminal/useDocumentTerminal.test.tsx @@ -33,6 +33,7 @@ import { ResourcesService, AmbiguousHostnameError, } from 'teleterm/ui/services/resources'; +import { IPtyProcess } from 'teleterm/sharedProcess/ptyHost'; import { WorkspaceContextProvider } from '../Documents'; @@ -88,15 +89,17 @@ const getDocTshNodeWithLoginHost: () => DocumentTshNodeWithLoginHost = () => { }; }; -const getPtyProcessMock = () => ({ +const getPtyProcessMock = (): IPtyProcess => ({ onOpen: jest.fn(), write: jest.fn(), resize: jest.fn(), dispose: jest.fn(), onData: jest.fn(), start: jest.fn(), + onStartError: jest.fn(), onExit: jest.fn(), getCwd: jest.fn(), + getPtyId: jest.fn(), }); test('useDocumentTerminal calls TerminalsService during init', async () => { diff --git a/web/packages/teleterm/src/ui/DocumentTerminal/useDocumentTerminal.ts b/web/packages/teleterm/src/ui/DocumentTerminal/useDocumentTerminal.ts index dcdac1db1ba7f..c65e170cba438 100644 --- a/web/packages/teleterm/src/ui/DocumentTerminal/useDocumentTerminal.ts +++ b/web/packages/teleterm/src/ui/DocumentTerminal/useDocumentTerminal.ts @@ -42,13 +42,13 @@ export function useDocumentTerminal(doc: types.DocumentTerminal) { const logger = useRef(new Logger('useDocumentTerminal')); const ctx = useAppContext(); const { documentsService } = useWorkspaceContext(); - const [attempt, startTerminal] = useAsync(async () => { + const [attempt, runAttempt] = useAsync(async () => { if ('status' in doc) { documentsService.update(doc.uri, { status: 'connecting' }); } try { - return await startTerminalSession( + return await initializePtyProcess( ctx, logger.current, documentsService, @@ -65,7 +65,7 @@ export function useDocumentTerminal(doc: types.DocumentTerminal) { useEffect(() => { if (attempt.status === '') { - startTerminal(); + runAttempt(); } return () => { @@ -73,12 +73,15 @@ export function useDocumentTerminal(doc: types.DocumentTerminal) { attempt.data.ptyProcess.dispose(); } }; + // This cannot be run only mount. If the user has initialized a new PTY process by clicking the + // Reconnect button (which happens post mount), we want to dispose this process when + // DocumentTerminal gets unmounted. To do this, we need to have a fresh reference to ptyProcess. }, [attempt]); - return { attempt, reconnect: startTerminal }; + return { attempt, initializePtyProcess: runAttempt }; } -async function startTerminalSession( +async function initializePtyProcess( ctx: IAppContext, logger: Logger, documentsService: DocumentsService, @@ -261,6 +264,8 @@ async function setUpPtyProcess( documentsService.update(doc.uri, { initCommand: undefined }); }; + // We don't need to clean up the listeners added on ptyProcess in this function. The effect which + // calls setUpPtyProcess automatically disposes of the process on cleanup, removing all listeners. ptyProcess.onOpen(() => { refreshTitle(); removeInitCommand(); @@ -278,6 +283,12 @@ async function setUpPtyProcess( // mark document as connected when first data arrives ptyProcess.onData(() => markDocumentAsConnectedOnce()); + ptyProcess.onStartError(() => { + if ('status' in doc) { + documentsService.update(doc.uri, { status: 'error' }); + } + }); + ptyProcess.onExit(event => { // Not closing the tab on non-zero exit code lets us show the error to the user if, for example, // tsh ssh cannot connect to the given node.