Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ message PtyEventOpen {}
message PtyEventExit {
uint32 exit_code = 1;
optional uint32 signal = 2;
string last_input = 3;
}

// PtyEventStartError is sent by the PTY process when the shared process fails to start it.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,6 @@ import { DuplexStreamingCall } from '@protobuf-ts/runtime-rpc';
import {
ManagePtyProcessRequest,
ManagePtyProcessResponse,
PtyEventData,
PtyEventResize,
PtyEventStart,
} from 'gen-proto-ts/teleport/web/teleterm/ptyhost/v1/pty_host_service_pb';

import {
Expand Down Expand Up @@ -56,7 +53,7 @@ export class PtyEventsStreamHandler {
await this.send({
event: {
oneofKind: 'start',
start: PtyEventStart.create({ columns, rows }),
start: { columns, rows },
},
});
}
Expand All @@ -65,7 +62,7 @@ export class PtyEventsStreamHandler {
await this.send({
event: {
oneofKind: 'data',
data: PtyEventData.create({ message: data }),
data: { message: data },
},
});
}
Expand All @@ -74,7 +71,7 @@ export class PtyEventsStreamHandler {
await this.send({
event: {
oneofKind: 'resize',
resize: PtyEventResize.create({ columns, rows }),
resize: { columns, rows },
},
});
}
Expand Down Expand Up @@ -110,7 +107,11 @@ export class PtyEventsStreamHandler {
}

onExit(
callback: (reason: { exitCode: number; signal?: number }) => void
callback: (reason: {
exitCode: number;
signal?: number;
lastInput: string;
}) => void
): RemoveListenerFunction {
return this.addDataListenerAndReturnRemovalFunction(
(event: ManagePtyProcessResponse) => {
Expand Down
8 changes: 7 additions & 1 deletion web/packages/teleterm/src/services/pty/ptyHost/ptyProcess.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,13 @@ export function createPtyProcess(
return stream.onOpen(callback);
},

onExit(callback: (reason: { exitCode: number; signal?: number }) => void) {
onExit(
callback: (reason: {
exitCode: number;
signal?: number;
lastInput: string;
}) => void
) {
return stream.onExit(callback);
},

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,8 @@ import {
ManagePtyProcessRequest,
ManagePtyProcessResponse,
PtyEventData,
PtyEventExit,
PtyEventOpen,
PtyEventResize,
PtyEventStart,
PtyEventStartError,
} from 'gen-proto-ts/teleport/web/teleterm/ptyhost/v1/pty_host_service_pb';

import {
Expand Down Expand Up @@ -75,44 +72,36 @@ export class PtyEventsStreamHandler {

private handleStartEvent(event: PtyEventStart): void {
this.ptyProcess.onData(data =>
this.stream.write(
ManagePtyProcessResponse.create({
event: {
oneofKind: 'data',
data: PtyEventData.create({ message: data }),
},
})
)
this.stream.write({
event: {
oneofKind: 'data',
data: { message: data },
},
})
);
this.ptyProcess.onOpen(() =>
this.stream.write(
ManagePtyProcessResponse.create({
event: {
oneofKind: 'open',
open: PtyEventOpen.create(),
},
})
)
this.stream.write({
event: {
oneofKind: 'open',
open: {},
},
})
);
this.ptyProcess.onExit(({ exitCode, signal }) =>
this.stream.write(
ManagePtyProcessResponse.create({
event: {
oneofKind: 'exit',
exit: PtyEventExit.create({ exitCode, signal }),
},
})
)
this.ptyProcess.onExit(payload =>
this.stream.write({
event: {
oneofKind: 'exit',
exit: payload,
},
})
);
this.ptyProcess.onStartError(message => {
this.stream.write(
ManagePtyProcessResponse.create({
event: {
oneofKind: 'startError',
startError: PtyEventStartError.create({ message }),
},
})
);
this.stream.write({
event: {
oneofKind: 'startError',
startError: { message },
},
});
});
// PtyProcess.prototype.start always returns a fulfilled promise. If an error is caught during
// start, it's reported through PtyProcess.prototype.onStartError. Similarly, the information
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,6 @@
*/

import { Struct } from 'gen-proto-ts/google/protobuf/struct_pb';
import {
CreatePtyProcessResponse,
GetCwdResponse,
} from 'gen-proto-ts/teleport/web/teleterm/ptyhost/v1/pty_host_service_pb';
import { IPtyHostService } from 'gen-proto-ts/teleport/web/teleterm/ptyhost/v1/pty_host_service_pb.grpc-server';

import Logger from 'teleterm/logger';
Expand Down Expand Up @@ -55,7 +51,7 @@ export function createPtyHostService(): IPtyHostService & {
callback(error);
return;
}
callback(null, CreatePtyProcessResponse.create({ id: ptyId }));
callback(null, { id: ptyId });
logger.info(`created PTY process for id ${ptyId}`);
},
getCwd: (call, callback) => {
Expand All @@ -69,8 +65,7 @@ export function createPtyHostService(): IPtyHostService & {
ptyProcess
.getCwd()
.then(cwd => {
const response = GetCwdResponse.create({ cwd });
callback(null, response);
callback(null, { cwd });
})
.catch(error => {
logger.error(`could not read CWD for id: ${id}`, error);
Expand Down
25 changes: 25 additions & 0 deletions web/packages/teleterm/src/sharedProcess/ptyHost/ptyProcess.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,4 +52,29 @@ describe('PtyProcess', () => {
);
});
});

if (process.platform !== 'win32') {
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kinda cowboyish way to avoid running this test on Windows. cmd.exe doesn't have its Ctrl+D equivalent and even if sh.exe is installed, it doesn't seem to work the same way on Windows.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kinda cowboyish way to avoid running this test on Windows

Yeah, looks like there's no cleaner way to skip a test conditionally.

test('including the last input in the exit event', async () => {
const pty = new PtyProcess({
path: 'sh',
env: {},
args: [],
useConpty: true,
ptyId: '1234',
});
await pty.start(80, 24);
const listener = jest.fn();
pty.onExit(listener);

pty.write('\x04');

await expect(() => listener.mock.calls.length > 0).toEventuallyBeTrue({
waitFor: 2000,
tick: 10,
});
expect(listener).toHaveBeenCalledWith(
expect.objectContaining({ lastInput: '\x04' })
);
});
}
});
8 changes: 6 additions & 2 deletions web/packages/teleterm/src/sharedProcess/ptyHost/ptyProcess.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ export class PtyProcess extends EventEmitter implements IPtyProcess {
private _logger: Logger;
private _status: Status = 'not_initialized';
private _disposed = false;
private _lastInput = '';

constructor(private options: PtyProcessOptions & { ptyId: string }) {
super();
Expand Down Expand Up @@ -115,6 +116,7 @@ export class PtyProcess extends EventEmitter implements IPtyProcess {
return;
}

this._lastInput = data;
this._process.write(data);
}

Expand Down Expand Up @@ -184,7 +186,9 @@ export class PtyProcess extends EventEmitter implements IPtyProcess {
return this.addListenerAndReturnRemovalFunction(TermEventEnum.Open, cb);
}

onExit(cb: (ev: { exitCode: number; signal?: number }) => void) {
onExit(
cb: (ev: { exitCode: number; signal?: number; lastInput: string }) => void
) {
return this.addListenerAndReturnRemovalFunction(TermEventEnum.Exit, cb);
}

Expand Down Expand Up @@ -229,7 +233,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, lastInput: this._lastInput });
this._logger.info(`pty has been terminated with exit code: ${e.exitCode}`);
this._setStatus('terminated');
}
Expand Down
2 changes: 1 addition & 1 deletion web/packages/teleterm/src/sharedProcess/ptyHost/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ export type IPtyProcess = {
onOpen(cb: () => void): RemoveListenerFunction;
onStartError(cb: (message: string) => void): RemoveListenerFunction;
onExit(
cb: (ev: { exitCode: number; signal?: number }) => void
cb: (ev: { exitCode: number; signal?: number; lastInput: string }) => void
): RemoveListenerFunction;
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -209,15 +209,12 @@ async function setUpPtyProcess(

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.
// tsh ssh couldn't connect to the given node.
//
// The downside of this is that if you open a local shell, then execute a command that fails
// (for example, `cd` to a nonexistent directory), and then try to execute `exit` or press
// Ctrl + D, the tab won't automatically close, because the last exit code is not zero.
//
// We can look up how the terminal in vscode handles this problem, since in the scenario
// described above they do close the tab correctly.
if (event.exitCode === 0) {
// We also have to account for Ctrl+D, as executing it makes the shell exit with the last
// reported exit code. If we depended on the exit code alone, it'd mean that the terminal tab
// wouldn't close if Ctrl+D followed a command that failed, say cd to a nonexistent directory.
if (event.exitCode === 0 || event.lastInput === /* Ctrl+D */ '\x04') {
documentsService.close(doc.uri);
}
});
Expand Down
Loading