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

Question: should .finalize be awaited? #772

Open
nicojs opened this issue Jul 4, 2024 · 5 comments
Open

Question: should .finalize be awaited? #772

nicojs opened this issue Jul 4, 2024 · 5 comments

Comments

@nicojs
Copy link

nicojs commented Jul 4, 2024

After upgrading eslint, I noticed that .finalize() returns a promise. I can see that this is also documented here: https://www.archiverjs.com/docs/archiver#finalize. However, it has yet to be awaited in the readme example. Can I simply await this promise instead of using the error and close event handlers?

To be clear:

This:

async function zip(from: string, to: string) {
	const output = fs.createWriteStream(to);
	const archive = archiver('zip', {
		zlib: { level: 9 }, // Sets the compression level.
	});
	archive.pipe(output);
	archive.directory(from, false);
	await archive.finalize();
	console.log(`✅ ${to} (${archive.pointer()} bytes)`);
}

Instead of this:

function zip(from: string, to: string) {
	return new Promise<void>((res, rej) => {
		const output = fs.createWriteStream(to);
		const archive = archiver('zip', {
			zlib: { level: 9 }, // Sets the compression level.
		});
		output.on('close', () => {
			console.log(`✅ ${to} (${archive.pointer()} bytes)`);
			res();
		});
		archive.on('error', err => rej(err));
		archive.pipe(output);
		archive.directory(from, false);
		archive.finalize();
      }
}
@Rabbitzzc
Copy link

same

@andrewlorenz
Copy link

My answer to this - after spending probably 20 hours trying to resolve an issue whereby my zips where either never finalized or corrupted, is NO ! Certainly not when you are piping to a stream. If you await the finalize then you are only going to get lucky if its called and archiver has already finished processing the files you gave it - i.e. a small number of small files. If you are zipping up anything more than a few KB you will almost certainly fall into the same hole I did, whereby the await never resolves.
Just set up your stream, pipe the archiver instance to it, feed it your files, and - in my case anyway - set the stream as the HTTP response. It will sort itself out nicely and just work as you'd hope . . .

@IvanUkhov
Copy link

IvanUkhov commented Oct 28, 2024

But if the destination is a file? In that case, one should await, right? It is still a stream, I suppose. But what should one do with the returned promise?

@jeff2013
Copy link

No you shouldn't await finalize. It seems to fail silently if you await finalize() and your code will exit without showing any errors. Very odd. I've tested this on a barebones js function in an AWS Lambda function. You should wrap your function in a promise and resolve when the "finish" event is fired from the archiver OR if you're passing in an uploadStream (to s3 for example), wait for the uploadStream to fire "close" or "finish".

@ciekawy
Copy link

ciekawy commented Jan 23, 2025

The issue with await archive.finalize() appears to come from a circular dependency in the archiver's internal design. The finalize() promise only resolves after the internal module emits its 'end' event, but this event depends on the output stream completing, which in turn requires the consumer to read the data. However, in many implementations (especially with HTTP responses or file streams), the consumer can't start reading until the function returns - which is blocked waiting for finalize().

If finalize() should not be awaited it should not return Promise. But actually there are reasons...

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

6 participants