Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Output streams do not emit "end" event if execution fails #517

Closed
anomiex opened this issue Feb 10, 2023 · 3 comments · Fixed by #518
Closed

Output streams do not emit "end" event if execution fails #517

anomiex opened this issue Feb 10, 2023 · 3 comments · Fixed by #518

Comments

@anomiex
Copy link

anomiex commented Feb 10, 2023

Consider this code (using node 18.13.0 and execa 6.1.0)

import { execa } from 'execa';

const proc = execa( "does not exist", [], { stdio: [ 'ignore', 'pipe', 'inherit' ], buffer: false } );

proc.stdout.on( 'end', () => console.log( 'stdout ended' ) );
proc.stdout.on( 'close', () => console.log( 'stdout closed' ) );

await proc.catch( e => e );

Only "stdout closed" is logged, "stdout ended" never happens.

This does not match the behavior of child_process:

import { spawn } from 'node:child_process';

const proc = spawn( "does not exist", [], { stdio: [ 'ignore', 'pipe', 'inherit' ] } );

proc.stdout.on( 'end', () => console.log( 'stdout ended' ) );
proc.stdout.on( 'close', () => console.log( 'stdout closed' ) );

proc.on( 'error', () => console.log( 'proc error' ) );

In this case both "stdout ended" and "stdout closed" fire as expected.

@anomiex
Copy link
Author

anomiex commented Feb 10, 2023

FYI, the code I'm trying to make work looks a bit more like this than the examples above:

import { execa } from 'execa';
import readline from 'readline';

async function* listFiles( dir ) {
    const proc = execa( 'git', [ 'ls-files' ], {
        cwd: dir,
        stdio: [ 'ignore', 'pipe', null ],
        buffer: false
    } );

    const rl = readline.createInterface( {
        input: proc.stdout,
        crlfDelay: Infinity,
    } );

    // We can't allow proc to reject, see https://github.com/sindresorhus/execa/issues/516#issuecomment-1426154012
    // So catch the error and save it to rethrow later.
    let err;
    proc.catch( e => { err = e; } );

    // Yield each line of output.
    yield* rl;

    // If proc rejects, throw the error now.
    await proc;
    if ( err ) throw err;
}

The failure case is if git somehow does not exist in the path.

If you have any better ways to handle this code that don't result in reading the whole output of the command into memory, I'd be happy for the suggestion. The rearrangement suggested by @ehmicky in #516 (comment) doesn't seem compatible with use of yield.

@ehmicky
Copy link
Collaborator

ehmicky commented Feb 11, 2023

Hi @anomiex,

Good catch! I have opened #518 to fix this problem when buffer is false.

@mrienstra
Copy link

Released in v7.0.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants