Mastodawn

Emelia 👸🏻Jul 18, 2024

This weirdness caused me to create like a million zero byte files in S3. So that was "fun”.

Emelia 👸🏻Jul 18, 2024

Turns out you need to await it with Promise.all() because otherwise errors just get eaten... because sure.

I'm beginning to think I've written this sub-optimally.. all I wanna do is download a file and stream it to s3.

samir, actually the xkcd top hat guy Jul 18, 2024

@thisismissem Does .pipe(…) help you at all?

@samir nope, because you can't create an Upload to S3 with a .pipe; It takes in a readable stream as an argument.

Markus Tacker Jul 19, 2024

@thisismissem @samir But you can pipe to that Readable stream, can't you?

@coderbyheart True, you could pass it a PassThrough and then pipe to its write side.

@samir @coderbyheart yeah, but something in the order of awaits means the upload gets zero bytes but everything else is fine

@thisismissem @coderbyheart I guess you might need to await on the “done” event of the input stream? I am really just guessing, though; no clue if this would help.

Events + promises = nightmare. I hope you can figure it out.

@samir @coderbyheart we were using the callback version of undici's stream method, but that caused uncaught exceptions which bullmq couldn't catch when executing the job, which caused an outage; I thought I's fixed it by awaiting the request instead, but that produced 0 byte uploads, and now I've a fix that awaits both request & uploads. It was properly frustrating to try to understand what the hell was happening here.

Markus Tacker Jul 19, 2024

@thisismissem @samir The way Node.js deals with Exceptions is definitely my main source of pain.

@coderbyheart @samir yeah, but also I think this is me not understanding this API.

Tempted to try to find a different way to write it.. but idk how because streams are event emitters and that's kinda incompatible with async/await

I could avoid the throws by doing a HEAD request first, I guess?

Jul 19, 2024

@thisismissem @coderbyheart @samir that would still leave (albeit tiny) race condition.

Maybe a not-as-efficient-streaming-but-better-API fetch() impl is a better intermediate choice.

@janl @coderbyheart @samir I'm also wondering if fetch() but with response.stream() might work?

I don't know. What I really wanna say is “start me an upload, and give me a writeable stream” then I can pipe the response to the writeable stream when I'm ready

Maybe I want to tee the stream instead of pipe'ing it?

@thisismissem @janl @coderbyheart I’d love to try and help further but this is a pretty difficult way of doing it. Do you feel like throwing up some code online in a REPL environment or something?

@samir @janl @coderbyheart here's the abstracted code: https://gist.github.com/ThisIsMissEm/b3977a76ca239b8c0328972e10ee4ea2

remote-file-upload-to-s3-undici.ts

GitHub Gist: instantly share code, notes, and snippets.

Gist

@thisismissem @janl @coderbyheart I'm a bit confused by this. It seems you have two things reading from the passthrough: the upload and the hasher. You're writing to it once, in the HTTP request.

I think you might need to explicitly clone the data across readers. Something like this: https://github.com/levansuper/readable-stream-clone/blob/master/src/readable-stream-clone.ts

If you remove the hasher (just as a test), does it start working?

(Also, I agree, Undici being both a streamer and a Promise is very odd. I don't like it.)

readable-stream-clone/src/readable-stream-clone.ts at master · levansuper/readable-stream-clone

Clone Readable Stream Multiple Times. Contribute to levansuper/readable-stream-clone development by creating an account on GitHub.

GitHub

Jul 19, 2024

@samir @thisismissem @coderbyheart I dont feel a clone is needed as in my head I built a pipeline: http req > hasher > counter > upload stream. But the code might not reflect that :)

@janl @thisismissem @coderbyheart The hasher doesn't go through to the upload, though; the pipe is being split, which means the data is being shared by two consumers. And I'm guessing the hasher always wins (perhaps because it's registered first?), which is why the upload ends up being 0 bytes.

I just saw there's a whole other thread of conversation in the comments. Shout if you'd like to move this discussion there.

Jul 19, 2024

@samir @thisismissem @coderbyheart hm, there is an original version of this code with a slightly different shape that had that same behaviour IIRC and it did the right thing (but didn’t allow errors to be thrown in the stream factory)