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

pipeline await and toBuffer #2975

Closed
kapouer opened this issue Nov 15, 2021 · 3 comments
Closed

pipeline await and toBuffer #2975

kapouer opened this issue Nov 15, 2021 · 3 comments

Comments

@kapouer
Copy link
Contributor

kapouer commented Nov 15, 2021

I am trying to pipe an input stream to a sharp pipeline and get a buffer out of it.
To be able to handle the input stream errors, i thought i could do this:

const sharp = require('sharp');
const pipeline = require('util').promisify(require('stream').pipeline);
const got = require('got');

async function getThumbnailBuffer(uri) {
	const pil = sharp().resize({
		fit: "inside",
		height: 64
	}).toFormat('webp', {
		quality: 50
	});
	const pipe = await pipeline(got.stream(uri), pil);
	const buf = await pil.toBuffer();
	return buf; // i know i can return pil.toBuffer(), i do this to show where we wait
};

however that doesn't work - await pipeline doesn't return when no error is thrown.
On the other hand, it correctly (meaning try { await getThumbnailBuffer(url); } works) throws an error (from got.stream) when there is one.

I also tried this but toBuffer is not a stream, so it can't work:

	return await pipeline(got.stream(uri), pil.toBuffer());

Instead i had to do that:

const sharp = require('sharp');
const pipeline = require('util').promisify(require('stream').pipeline);
const got = require('got');

module.exports = async function (uri) {
	const pil = sharp().resize({
		fit: "inside",
		height: 64
	}).toFormat('webp', {
		quality: 50
	});
	let toBuff;
	setTimeout(async () => {
		toBuff = pil.toBuffer();
	});
	await pipeline(got.stream(uri), pil);
	const buf = await toBuff;
	return buf; // i know i can return toBuff, i do this to show where we wait
};

this works and also catches (in a way compatible with async/await) errors.
However it's ugly.
It's hard to tell if it comes from

  • node
  • got
  • sharp
    but right now i'm inclined to think the need to call "toBuffer" to trigger the stream is odd.
@lovell
Copy link
Owner

lovell commented Nov 15, 2021

A sharp instance implements a Duplex stream so won't emit the close event (and therefore resolve the "promisified" logic here) until it knows the destination of the output, as this might be another WritableStream.

Perhaps an alternative approach might be to await the input and output sides concurrently, something like (untested):

const [pipe, buf] = await Promise.all([
  pipeline(got.stream(uri), pil),
  pil.toBuffer()
]);

@kapouer
Copy link
Contributor Author

kapouer commented Nov 15, 2021

Yes ! that's so much nicer ! Even better, this works too:

const [buf] = await Promise.all([
  pil.toBuffer(),
  pipeline(got.stream(uri), pil)
]);

maybe it would help so much if it was given as an example somewhere in the documentation.
EDIT: i made sure this actually works and also actually throws correctly in case of input stream error.

@lovell
Copy link
Owner

lovell commented Dec 12, 2021

Glad you got it working. I've tagged this as cookbook for inclusion in future possible document relating to problem-oriented solutions (as opposed to API-level reference).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants