sharp icon indicating copy to clipboard operation
sharp copied to clipboard

Converting the Sharp/Node Duplex to web standard ReadableStream

Open FunctionDJ opened this issue 1 year ago • 7 comments

Question about an existing feature

What are you trying to achieve?

I'm trying to convert a Sharp object as returned by sharp("...").webp() to a web standard ReadableStream<Uint8Array> for compatibility with Deno.serve() / new Response(body)

When you searched for similar issues, what did you find that might be related?

I have searched a lot, e.g. in the "issues" tab on this repo, but couldn't find any truly related issue. (Aside from #4013 maybe) I managed to make it work with:

  • new ReadableStream: new ReadableStream({ start(controller) { sharpObject.on("data", chunk => controller.enqueue(chunk) } }) - but it's slightly slower than the classic sharpObject.pipe(res) you'd use with http.createServer or express
  • Readable.toWeb(sharpObject) - appears to pipe some image data, but the served image appears to be broken/blank and it's significantly slower (~3x) on a 20MB input file than .pipe(res) or the previous new ReadableStream option (Note: toWeb was added in Node v17 and is still marked as experimental)
  • createReadableStreamFromReadable from @remix-run/node works but is ~3x as slow as .pipe(res). They also use their own new ReadableStream. This function can be used with import { createReadableStreamFromReadable } from "@remix-run/node" even when not using the Remix framework.

Somewhat related: Stackoverflow posts about the difference between web standard ReadableStream and Node Readable

  • https://stackoverflow.com/a/66629140/9948553
  • https://stackoverflow.com/q/37614649/9948553

Please provide a minimal, standalone code sample, without other dependencies, that demonstrates this question

const sharpObject = sharp("img.png").webp();
const readableStream: ReadableStream<Uint8Array> = sharpObject;
/**
 * TypeScript will already warn here that
 * ` 'Sharp' is missing the following properties from type 'ReadableStream<Uint8Array>' `,
 * which at runtime holds true as the `readableStream` variable doesn't work
 * with e.g. `new Response(readableStream)` in Deno
 */

Please provide sample image(s) that help explain this question

independent of image file

FunctionDJ avatar Oct 12 '24 00:10 FunctionDJ

Readable.toWeb(sharpObject) - appears to pipe some image data, but the served image appears to be broken/blank

Use of toWeb is probably what I'd suggest people try first when converting. Are you able to provide a minimal code sample that allows someone else to reproduce the problem you ran into?

lovell avatar Oct 12 '24 09:10 lovell

@lovell I'm using a large, 20MB PNG file for these tests, with only 1 request at a time. I haven't tested multiple/mass requests yet and it would make the reproduction more complicated but mass requests is my actual use case which is why i'm considering performance.

This first example works both when run with Deno and with Node and has equal performance between the two JS runtimes on average in my test case:

import http from "node:http";
import sharp from "sharp";

http
	.createServer((_req, res) => {
		const sharpObject = sharp("img.png").webp();
		sharpObject.pipe(res);
	})
	.listen(8000);

This second example only works in Deno and is about 1% slower on average. More importantly, the response appears to be incomplete. Chromium will log GET http://localhost:8000/ net::ERR_INCOMPLETE_CHUNKED_ENCODING 200 (OK) in the console and the bottom ~15% of the image appear as gray and Postman reports ~800KB response size vs. the 1MB response of the first example.

import sharp from "npm:sharp";
import { Readable } from "node:stream";

Deno.serve(() => {
	const sharpObject = sharp("img.png").webp();
	return new Response(Readable.toWeb(sharpObject) as ReadableStream);
});

With the original post and the 3x performance difference i've mentioned, i wasn't able to reproduce this today so i must have made a mistake in input files when i did the previous testing. Please excuse me.

FunctionDJ avatar Oct 14 '24 20:10 FunctionDJ

This looks a bit like the problem reported at https://github.com/denoland/deno/issues/24606

If performance is of concern and memory is not a limiting factor, perhaps experiment with Buffers rather than Streams to avoid many small memory allocations.

lovell avatar Oct 14 '24 20:10 lovell

I found that Deno issue too, but i'm not using the Fresh framework or http2 (request protocol is http/1.1 according to Chromium Devtools).

As for Buffers, i'm not aware that they can be streamed. If i use sharpObject.toBuffer(), then the code will wait for the conversion to finish, keeping all data in memory for the moment (if i understood correctly).

I've also talked to contributors on the Deno side and expectedly they told me to ask on the package (sharp) side to look into this issue.

FunctionDJ avatar Oct 14 '24 20:10 FunctionDJ

Bun is working with the following code

import sharp from "sharp"
import { Readable } from "node:stream"
import { Buffer } from "node:buffer"

Bun.serve({
  fetch: async (req) => {
    const svg = await req.text()
    const sharpObject = sharp(Buffer.from(svg)).webp()
    return new Response(Readable.toWeb(sharpObject) as ReadableStream, {
      headers: {
        'Content-Type': 'image/webp',
      },
    })
  },
})

But it getting this error message for each request, not sure why it occurs

error: Premature close
 code: "ERR_STREAM_PREMATURE_CLOSE"

      at new NodeError (node:stream:244:20)
      at node:stream:714:47
      at emit (node:events:48:48)
      at emit (node:events:48:48)
      at endReadableNT (node:stream:2207:27)
      ```

ducan-ne avatar Oct 24 '24 07:10 ducan-ne

@FunctionDJ Were you able to make any progress with this?

lovell avatar Jan 27 '25 08:01 lovell

@lovell No. The project that caused me to write this issue is in hiatus and i might rewrite it in Go for performance reasons that are unrelated to sharp - aside from this particular issue in combination with Deno.

If you're asking because you'd like to close abandoned issues, it's your project, but i think "native" WHATWG compatibility might come up again.

FunctionDJ avatar Jan 27 '25 09:01 FunctionDJ