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

Feature Request: Emit error on early 7z process termination #105

Open
sysrage opened this issue Aug 5, 2021 · 9 comments
Open

Feature Request: Emit error on early 7z process termination #105

sysrage opened this issue Aug 5, 2021 · 9 comments

Comments

@sysrage
Copy link

sysrage commented Aug 5, 2021

Currently, if the spawned 7-zip process is killed before extraction is complete, the 'end' event is emitted and no errors are shown. Since the extraction actually failed in this case, an 'error' event should be emitted instead.

If there's some technical reason this shouldn't be fixed, please update documentation to suggest that people implement their own error handling for this corner-case. Something like:

extractStream.on('end', () => {
  if (!extractStream.info.Compressed) {
    return reject(new Error('7za Process Terminated Early'));
  }
  resolve();
});

extractStream.on('error', (error) => {
  reject(error);
});
@sysrage
Copy link
Author

sysrage commented Aug 5, 2021

It looks like the above workaround doesn't even work. Sometimes extracStream.info.Compressed exists and sometimes it doesn't. Why doesn't this always exist when the 'end' event is fired and extraction was complete?

@q2s2t
Copy link
Owner

q2s2t commented Aug 5, 2021

Which method do you use for killing the process? Ctrl+C ?

@sysrage
Copy link
Author

sysrage commented Aug 5, 2021

For my initial testing, I was clicking 'End Process' in Task Manager. Based on your question, I'll see if other termination signals result in a properly emitted error!

Or not...

taskkill /im 7za.exe
ERROR: The process "7za.exe" with PID 51192 could not be terminated.
Reason: This process can only be terminated forcefully (with /F option)

@sysrage
Copy link
Author

sysrage commented Aug 5, 2021

Can you help me understand why extractStream.info is intermittently missing several properties when the 'end' event is emitted after a successful extract?

This is what I see after successfully extracting one file:

extractStream.info Map(9) {
  '7-Zip (a) 19.00 (x64) ' => 'Copyright (c) 1999-2018 Igor Pavlov : 2019-02-21',
  'Extracting archive' => 'C:\\SomeDirectory\\myarchive.7z',
  'Path' => 'C:\\SomeDirectory\\myarchive.7z',
  'Type' => '7z',
  'Physical Size' => '3283117249',
  'Headers Size' => '1340',
  'Method' => 'LZMA2:30 LZMA:20 BCJ2',
  'Solid' => '+',
  'Blocks' => '11'
}

This is what I see when extracting another file:

extractStream.info Map(13) {
  '7-Zip (a) 19.00 (x64) ' => 'Copyright (c) 1999-2018 Igor Pavlov : 2019-02-21',
  'Extracting archive' => 'C:\\SomeDirectory\\myarchive2.7z',
  'Path' => 'C:\\SomeDirectory\\myarchive2.7z',
  'Type' => '7z',
  'Physical Size' => '1462534299',
  'Headers Size' => '19534',
  'Method' => 'LZMA2:24 LZMA:19 BCJ2',
  'Solid' => '+',
  'Blocks' => '3',
  'Folders' => '5',
  'Files' => '2907',
  'Size' => '1524469438',
  'Compressed' => '1462534299'
}

Why are Folder, Files, Size, and Compressed missing?

@sysrage
Copy link
Author

sysrage commented Aug 6, 2021

So, the above behavior is not specific to a particular archive. It appears to be a timing issue, as the same archive file will contain proper info data sometimes but not every time. If I have other disk I/O active while the extraction is in progress, it seems to increase the time between the 'end' event is emitted and the info map is fully populated.

@sysrage
Copy link
Author

sysrage commented Aug 12, 2021

Thanks for the thumbs-up on the previous comment, but can you please provide any input on anything reported?

Related to this issue, is there any method currently available for either of the following?

  • Obtaining the PID of the 7-zip process that was launched to extract the file. This could be used elsewhere to kill said process, when an abort signal is received, rather than killing all 7za processes.
  • Cancelling the extraction when some sort of abort signal is emitted. This could be either an AbortController implementation or something like axios' cancelToken implementation.

I'm attempting to use this package for a project that will be extracting very large archives and want to allow users to cancel, if necessary.

EDIT: After posting this, I realized I can likely use $childProcess so I know the PID, but that's not very convenient. Would it be possible to add the 7-zip PID to stream.info for easy access when not using $childProcess? Even better would be an integrated abort implementation!

@q2s2t
Copy link
Owner

q2s2t commented Aug 12, 2021

I will work on that in the next few weeks (no access to a computer for now)

This was referenced Feb 2, 2022
@q2s2t
Copy link
Owner

q2s2t commented Feb 6, 2022

Can @sysrage confirm that commit e961ac4 fixes the issue?

@sysrage
Copy link
Author

sysrage commented Feb 7, 2022

Using commit e961ac4, the original issue still exists. No error event is emitted if the 7-zip process is killed during extraction. The end event is emitted, as-if there were no errors. So, I still submit a request to emit an error event if the 7-zip process ends unexpectedly.

Additionally, the extractStream.info property seems to always be missing the Folder, Files, Size, and Compressed properties now. As mentioned above, with the released build it would intermittently include these properties upon successful extraction.

Support for an AbortController or some other cancellation method would also still be very useful.

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

No branches or pull requests

2 participants