raunioroo•13mo ago

await writer.write(buffer) never resolves on Deno.Command

Hiho. I'm creating a Deno.Command that writes a buffer to vipsthumbnail via stdin (and reads the result via stdout). It works most of the time, but sometimes gets stuck on await writer.write(source);. Source is the result of await Deno.readFile(path). The big problem is that there is no error, the writer.write() promise is just never resolved. So I have no idea how to debug, catch errors or even cancel this after timeout or something. I can process a couple of files (all with a new Deno.Command) but then after a few files it gets stuck and the promise never resolves. Any ideas how to begin solving what the problem is.

    const cmd = new Deno.Command(this.vipspath + this.vipsthumbnail, {
      args: args,
      stdin: "piped",
      stderr: "piped",
      stdout: "piped",
    }).spawn();

    const writer = cmd.stdin.getWriter();
    console.log("START WRITING", typeof source, source.byteLength);
    await writer.write(source); // <--- STUCK HERE MAY NEVER RESOLVE
    console.log("YAY! WE GOT HERE");
    await writer.ready;
    await writer.close();

    const cmd = new Deno.Command(this.vipspath + this.vipsthumbnail, {
      args: args,
      stdin: "piped",
      stderr: "piped",
      stdout: "piped",
    }).spawn();

    const writer = cmd.stdin.getWriter();
    console.log("START WRITING", typeof source, source.byteLength);
    await writer.write(source); // <--- STUCK HERE MAY NEVER RESOLVE
    console.log("YAY! WE GOT HERE");
    await writer.ready;
    await writer.close();

18 Replies

Joe Hildebrand•13mo ago

Are you sure that vipsthumbnail is actually reading stdin if stdin hasn't closed? If the output buffer fills up, writer.write() will block. you could try await Promise.all([writer.write(), writer.close()]) to find out

rauniorooOP•13mo ago

hm. no I'm not sure :) it appears to read, I can process a couple of files just fine. is it possible vipsthumbnail does read stdin, but then stops reading it when it encounters some kind of error parsing a certain file?

Joe Hildebrand•13mo ago

anything's possible, of course. All programs have bugs.

rauniorooOP•13mo ago

this doesnt seem to help with getting it unstuck

Joe Hildebrand•13mo ago

however, when vipsthumbnail exits without reading pending stdin, I would have expected await writer.write to throw.

rauniorooOP•13mo ago

the weird thing is that vipsthumbnail prints stuff to stderr, like _file "" does not exist, but then it still gives a proper result. maybe it first tries to open a file named "stdin", prints some errors, and only then reverts to the code path that actually reads stdin. (you are supposed to call it with "stdin" in place of the input file name argument)

jeff.hykin•13mo ago

might be unrelated but this sounds similar to this old problem https://discord.com/channels/684898665143206084/1074484890562736188/1074682730073313340

rauniorooOP•13mo ago

how would I go about writing a timeout to abort the writer.write() if it takes x amount of time

Joe Hildebrand•13mo ago

What if you throw cmd.status() into the Promise.all? Do you get a zero exit code?

rauniorooOP•13mo ago

await Promise.all([writer.write(source), writer.close(), cmd.status]); just makes it fail on the first file (same result, never resolves)

jeff.hykin•13mo ago

This uses Deno.run but it should be the same general logic https://github.com/jeff-hykin/quickr/blob/139ebd639bb474be1b0b77a3c3f273ca36c93d6d/main/run.js#L592C23-L592C23

GitHub

quickr/main/run.js at 139ebd639bb474be1b0b77a3c3f273ca36c93d6d · je...

💾 📦 Tools for Deno. Contribute to jeff-hykin/quickr development by creating an account on GitHub.

rauniorooOP•13mo ago

Copilot to the rescue, it wrote this nicely working timeout function, so at least I can move on when it fails.

    const timeout = new Promise((_, reject) =>
      setTimeout(() => reject(new Error("Operation timed out")), 1000)
    );
    try {
      await Promise.race([writer.write(source), timeout]);
    } catch (error) {
      console.log("writing to vipsthumbnail timed out");
      return null;
    }

    const timeout = new Promise((_, reject) =>
      setTimeout(() => reject(new Error("Operation timed out")), 1000)
    );
    try {
      await Promise.race([writer.write(source), timeout]);
    } catch (error) {
      console.log("writing to vipsthumbnail timed out");
      return null;
    }

jeff.hykin•13mo ago

did you not actually want to cancel the process on timeout?

rauniorooOP•13mo ago

oh yeah I should do that, close everything before returning

jeff.hykin•13mo ago

okay. if you try to kill a process that already stopped it'll get an error. The code I linked handles that case, although you'll have to integrate it with the write-timeout code

rauniorooOP•13mo ago

When it works, it prints stuff like this to stderr, but does still give a beautiful image back :)

/usr/bin/vipsthumbnail: unable to thumbnail 
VipsForeignLoad: file "" does not exist
/usr/bin/vipsthumbnail: unable to thumbnail 

VipsForeignLoad: file "" does not exist

/usr/bin/vipsthumbnail: unable to thumbnail 
VipsForeignLoad: file "" does not exist
/usr/bin/vipsthumbnail: unable to thumbnail 

VipsForeignLoad: file "" does not exist

would like to avoid using the deprecated Deno.run if at all possible

jeff.hykin•13mo ago

the code will be basically the the same for Deno.Commad. There's a .kill() method and an .status() promise

rauniorooOP•13mo ago

I think this is kinda solved (I can now process images fine most of the time and "cleanly" abort when it fails for whatever reason). I learned a lot more than expected:) Thank you both! this was probably the most key piece of info. not 100% sure but I'm assuming vips stops reading stdin when it encounters.... something, and then this happens wait a sec. actually all my problems went away with this much shorter and leaner solution: instead of buffer source is now ReadableStream (which is nicer already) Then I just use source.pipeTo(cmd.stdin). Apparently pipeTohandles things like backpressure in a way that makes vipsthumbnailer stay happy on the receiving end. And this further confirms that vips wasn't failing on some files, just my way of feeding it (I guess the earlier writer.write method only worked for very small files or something).

    const cmd = new Deno.Command(this.vipspath + this.vipsthumbnail, {
      args: args,
      stdin: "piped",
      stderr: "piped",
      stdout: "piped",
    }).spawn();

    source.pipeTo(cmd.stdin);
    const out = await cmd.output();

    const cmd = new Deno.Command(this.vipspath + this.vipsthumbnail, {
      args: args,
      stdin: "piped",
      stderr: "piped",
      stdout: "piped",
    }).spawn();

    source.pipeTo(cmd.stdin);
    const out = await cmd.output();

For full disclosure if anyone attempts similar stuff with vips. Spent like 6 full days to integrate vips, trying so many different approaches. It was a proper adventure. Firstly: wasm. So, wasm-vips sounds nice but seems not ready yet. Main reasons: leaks memory like crazy, is fast but slower than native cli, except for AVIF which is just suuuper-slow on wasm. Also it seems deno version is not quite at the level of node version (importing node version to deno through npm compat does not work, tried that too). Also I somehow thought using wasm would be the "pure/clean" approach, but did a 180 on that ideologically, and now think that one should use native apps whenever possible. Trouble with native vips is that it is very painful to install on Amazon's own AWS-optimized linux distro (no prebuilt binaries and even getting compiling to work is a frustrating experience). Solution here is to just switch to Ubuntu, which has vips ready among many other niceties. Using native cli it is then. Passing data through STDIN as described above kinda works, but is unreliable for some reason and somewhat rarely but randomly fails in mysterious ways (even the pipeTo approach). Using just plain local file paths and/or temporary files is 100% reliable. Back to basics I guess :) I was dead set on trying non-file-backed piped approach to passing stuff to, and back from vips, as my files come from various non-file sources like S3. Luckily I realized I run every request to external services through a local file-based caching setup, so I do in fact have a local file path for everything after all, so no need to use temp files. Just need to ask the caching layer for the secret path to read from :) STDOUT seems to work reliably for getting the converted files back to deno, no problem there