diff --git a/tests/index.test.ts b/tests/index.test.ts index c2b484d..50496f6 100644 --- a/tests/index.test.ts +++ b/tests/index.test.ts @@ -47,6 +47,7 @@ function getOpenFds(): FdRecord { describe( 'PTY', + { repeats: 50 }, () => { test('spawns and exits', () => new Promise((done) => { @@ -273,6 +274,26 @@ describe( }); })); + test('resize after close shouldn\'t throw', () => new Promise((done, reject) => { + const pty = new Pty({ + command: '/bin/sh', + onExit: (err, exitCode) => { + try { + expect(err).toBeNull(); + expect(exitCode).toBe(0); + } catch (e) { + reject(e) + } + }, + }); + + pty.close(); + expect(() => { + pty.resize({ rows: 60, cols: 100 }); + }).not.toThrow(); + done(); + })); + test( 'ordering is correct', () => @@ -302,11 +323,11 @@ describe( buffer = Buffer.concat([buffer, data]); }); }), - { repeats: 1 }, ); testSkipOnDarwin( 'does not leak files', + { repeats: 4 }, () => new Promise((done) => { const oldFds = getOpenFds(); @@ -348,11 +369,11 @@ describe( done(); }); }), - { repeats: 4 }, ); test( 'can run concurrent shells', + { repeats: 4 }, () => new Promise((done) => { const oldFds = getOpenFds(); @@ -414,7 +435,6 @@ describe( done(); }); }), - { repeats: 4 }, ); test("doesn't break when executing non-existing binary", () => @@ -434,7 +454,6 @@ describe( } })); }, - { repeats: 50 }, ); describe('cgroup opts', () => { diff --git a/wrapper.ts b/wrapper.ts index e4796ea..fe3d5f9 100644 --- a/wrapper.ts +++ b/wrapper.ts @@ -85,12 +85,14 @@ export class Pty { } this.#fdEnded = true; - exitResult.then((result) => realExit(result.error, result.code)); + exitResult.then((result) => { + realExit(result.error, result.code) + }); userFacingRead.end(); }; this.#socket.on('close', handleClose); - // PTYs signal their donness with an EIO error. we therefore need to filter them out (as well as + // PTYs signal their done-ness with an EIO error. we therefore need to filter them out (as well as // cleaning up other spurious errors) so that the user doesn't need to handle them and be in // blissful peace. const handleError = (err: NodeJS.ErrnoException) => { @@ -103,11 +105,11 @@ export class Pty { return; } if (code.indexOf('EIO') !== -1) { - // EIO only happens when the child dies . It is therefore our only true signal that there + // EIO only happens when the child dies. It is therefore our only true signal that there // is nothing left to read and we can start tearing things down. If we hadn't received an // error so far, we are considered to be in good standing. this.#socket.off('error', handleError); - this.#socket.destroy(); + this.#socket.end(); return; } } @@ -118,7 +120,9 @@ export class Pty { } close() { - this.#socket.destroy(); + // end instead of destroy so that the user can read the last bits of data + // and allow graceful close event to mark the fd as ended + this.#socket.end(); } resize(size: Size) { @@ -126,7 +130,18 @@ export class Pty { return; } - ptyResize(this.#fd, size); + try { + ptyResize(this.#fd, size); + } catch (e: unknown) { + if (e instanceof Error && 'code' in e && e.code === 'EBADF') { + // EBADF means the file descriptor is invalid. This can happen if the PTY has already + // exited but we don't know about it yet. In that case, we just ignore the error. + return; + } + + // otherwise, rethrow + throw e; + } } get pid() {