ffbinaries/ffbinaries-node

execSync write error when checking version

Closed this issue · 3 comments

I just ran into an unusual situation, but did come up with a remedy for it. I was playing with deploying my ffmpeg-streamer app on a remote ubuntu server and ran into a little trouble.

I made a respawn app to keep track of ffmepg-streamer which will respawn it 10 seconds after if it exits due to crashing or somebody pressing the x button on it. Works surprisingly well and only took about 5 minutes to make. After running the process in the command line and sending it to the background with a trailing & (./respawn-linux-x64 ./ffmpeg-streamer-linux-x64 3000 &) and exiting the ssh terminal, it seemed that some permissions were lost and execSync could no longer check the version. Returned some type of cryptic EIO i/o write error.

I dug deep and tried many options with various versions of child processes such as execFileSync and spawnSync. The problem seems to originate from execSync and execFileSync using a shell, while spawnSync does not.

For line 412, I propose to make a small change which seems a little cleaner than the try catch:

    // check version
    if (result.found && result.isExecutable) {
      var stdout = childProcess.spawnSync(result.path, ['-version']).stdout;
      // if stdout.length === 0, then we must have stderr instead
      if (stdout.length) {
        result.version = stdout.toString().split(' ')[2];
      }
    }

spawnSync actually returns an object with properties such as output, stdout, stderr, etc. I just check stdout.length to make sure it is filled with something other than an empty buffer array of length 0. Verified this works for me on windows, mac, and ubuntu 17.

If this looks good to you, I could just make a PR. Let me know. Thanks.

vot commented

Hi @kevinGodell,

I'm perfectly happy with spawnSync being used there and if that covers the edge cases a little better then no reason not to do it.

A pull request would be great, thank you!

vot commented

Version 1.0.9 contains the changes and has been released to npm.

Closing issue.

I incorporated 1.0.9 into my app an it works as planned. Thanks for including the pull.