Adds UnpackStdout | Fixes not waiting for stdio streams to close

[honestabelink]

Adds initial support for unpacking over stdout.

Fixes early 'exit' callbacks while stdio streams still contain data.

https://nodejs.org/api/child_process.html#child_process_event_close
The 'close' event is emitted after a process has ended and the stdio streams of a child process have been closed

Addresses the possibility of invoking handler functions multiple times.

https://nodejs.org/api/child_process.html#child_process_event_error
When listening to both the 'exit' and 'error' events, guard against accidentally invoking handler functions multiple times.

[onikienko]

Hi @honestabelink,

Thank you for your contribution.

May you please explain what it fixes? You have replaced error with close. So we have no error listener.

Docs says: The 'exit' event may or may not fire after an error has occurred. When listening to both the 'exit' and 'error' events, guard against accidentally invoking handler functions multiple times.

Possible you were about to add close in addition to our exit and error (or replace exit with close) to handle the situation when multiple processes might share the same stdio streams?
Why do we need to wait for close?

[honestabelink]

Hello @onikienko ,

Sorry about this, the intent was surprisingly to remove 'error' and change 'exit' to 'close'.
surprisingly only in the sense I can't believe how much my brain shut off

Also sorry for not including all the relevant information.

https://nodejs.org/api/child_process.html#child_process_event_exit
When the 'exit' event is triggered, child process stdio streams might still be open.

There still may be data in the stdio streams when exit is called, meaning your cb callback will return incomplete results.
You must wait for stdio streams to finish.

https://nodejs.org/api/child_process.html#child_process_event_close
The 'close' event is emitted after a process has ended and the stdio streams of a child process have been closed. This is distinct from the 'exit' event, since multiple processes might share the same stdio streams

_7z.cmd(['x', '-so', dbArchive.path, f[0].name,], (error, results) => {...
-so : write to stdout

For this command, 'exit' was being called before all data coming from 7z was process by proc.stdout.on('data'.... Again, this results in incomplete data being passed to the callback. Using 'close' ensures all stdio streams have been completed and all data has been processed by proc.stdout.on('data'..., and as a bonus it catches errors too.

And yes 'exit' and 'error' should be not listened for together, unless you track it, to know the callback should be skipped the second time.

Thanks for your time.
Feel free to close this. If you'd like feel free to make the changes yourself, or I'll send another pull request.

[onikienko]

Thank you for your detailed answer @honestabelink

Possible the issue you have with

_7z.cmd(['x', '-so', dbArchive.path, f[0].name,], (error, results) => {...

is coming from this line

7zip-min/index.js

Line 60 in 6dcc96e

let result = null;

I think we should have this here:

let result = output;

Looks like the current code return output only for l (list command). And mentioned _7z.cmd command will always have null as result. May you please check?

[honestabelink]

@onikienko

Yes but, sorry more relevant information left out

I've added additional functionality to extract files over stdout and handle the binary data. unpublished on my branch
When handling large files being extracted 'exit' is sometimes called before all data from 7z is processed.
Changing 'exit' to 'close' as per the documentation, corrects the issue and I get the full extracted files.

'close' unlike 'exit' is called only after the stdio streams have been closed. This ensures all stdio streams have been flushed and all data has been processed by any listeners ,proc.stdout.on('data', (chunk) => {... before calling our callback routines.

I've been doing this to handle both cases.

function run(bin, args, cb) {
  cb = onceify(cb);
  const proc = spawn(bin, args);
  let output = []

  proc.on('close', function (code) {
    let result = null;
    if (args[0] === 'l') {
      result = parseListOutput(Buffer.from(output).toString())
    } else if (args[0] === 'x') {
      result = output
    }
    cb(code ? new Error('Exited with code ' + code) : null, result);
  });

  proc.stdout.on('data', (chunk) => {
    output.push(...chunk)
  });
}

[onikienko]

Got your point @honestabelink

You are right. Please send PR if possible. I will publish it with next release.

[onikienko]

Thank you @honestabelink.

I think I will merge this PR to the release-2.0.0 branch. And release once this milestone https://github.com/onikienko/7zip-min/milestone/1 will be done.