When using tar-fs and some files cannot be read due to e.g. EACCES the resulting pack() stream does not emit an error event.
After tracing it down, I found that in pack.js:169 self.destroy() is called without providing an error argument. This causes Pack.destroy(), lines 198-205 to emit a 'close' event, but also sets the internal this._destroyed variable to true, causing future calls to not emit anything anymore (which seems reasonable as close has to be the last event). tar-fs is provided the error in the callback called in pack.js:170 and this callback will, inside the tar-fs project, again trigger a Pack.destroy(error) call. Of course this will then be ignored.
Even worse, as a test I passed the error in pack.js:169 which eventually gave me a 'error' event correctly, altough the error itself was 'premature exit'. I hope this does not go too much off topic, as this matter is overarching over both projects.
My expectation when using tar-fs would be:
If any file cannot be read, the resulting tar stream should give an error event, not just a close event
This error event is either directly the error of the file read stream or a general error containing the original error as detail
It would also be nice that in the case a file is not readable at all (e.g. EACCES) there would be an option that this file can be skipped. To implement that, we could delay writing the header data of that file to the stream until the first bytes were successfully read.
To enable above functionality I think we need either more fine-grained control over the error handling in tar-stream or at least pass correct errors.
Is there a reason why in the first destroy() call we don't pass the real error values? Passing error values there would probably solve this problem already to a great degree. There are also some other parts which call destroy() in error cases without passing errors.
I would be happy to provide PRs as soon as we know how to solve this!
mafintosh
commented
1 year ago
When using tar-fs and some files cannot be read due to e.g. EACCES the resulting
pack()stream does not emit an error event.After tracing it down, I found that in pack.js:169
self.destroy()is called without providing an error argument. This causesPack.destroy(), lines 198-205 to emit a'close'event, but also sets the internalthis._destroyedvariable to true, causing future calls to not emit anything anymore (which seems reasonable as close has to be the last event). tar-fs is provided the error in the callback called in pack.js:170 and this callback will, inside the tar-fs project, again trigger aPack.destroy(error)call. Of course this will then be ignored.Even worse, as a test I passed the error in pack.js:169 which eventually gave me a
'error'event correctly, altough the error itself was 'premature exit'. I hope this does not go too much off topic, as this matter is overarching over both projects.My expectation when using tar-fs would be:
To enable above functionality I think we need either more fine-grained control over the error handling in tar-stream or at least pass correct errors.
Is there a reason why in the first
destroy()call we don't pass the real error values? Passing error values there would probably solve this problem already to a great degree. There are also some other parts which calldestroy()in error cases without passing errors.I would be happy to provide PRs as soon as we know how to solve this!