mscdex / busboy

A streaming parser for HTML form data for node.js
MIT License
2.84k stars 213 forks source link

feat: emit complete event #326

Closed atian25 closed 1 year ago

atian25 commented 1 year ago

I'm writing a module to wrap busboy with async generator, and found a need to emit a complete event when busybox parse all fields and files, please take a look, thanks.

mscdex commented 1 year ago

This shouldn't be necessary. You should see a 'close' event when the busboy instance's end() is called (which happens automatically when piping another stream to busboy).

atian25 commented 1 year ago

when using async generator but don't consume stream in it, then the close won't trigger.

I think parse complete event is one of the lifecycle

atian25 commented 1 year ago

will upload a fixture case later

mscdex commented 1 year ago

If you're not consuming file streams (even if ignoring the contents), you may not get this 'complete' event either, depending on the total size of the request and node's stream buffering (highWaterMark). Therefore I'd suggest simply calling at least file.resume() to skip over files you're not interested in.

atian25 commented 1 year ago

https://github.com/atian25/busboy/pull/1/files

some test fixtures.

mscdex commented 1 year ago

Like I said, you have to do something with the data for each file in the request, whether you buffer it, discard it, or stream it somewhere else. That's just the nature of parsing data in general.

If you're trying to "use" each file's stream later (after the entire form is parsed) that inherently means you're buffering all of the data in memory (potentially dangerous if not limited) or temporarily storing it on disk for example. busboy will not force those kinds of decisions on you, so it's up to you to handle it how you like.