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

handle unpipeing, cancelling or destroying of busboy accordingly #81

Merged
merged 1 commit into from
Jun 9, 2022

Conversation

Uzlopak
Copy link
Contributor

@Uzlopak Uzlopak commented Jan 5, 2022

Should fix fastify/fastify-multipart#315

Checklist

@kibertoad
Copy link
Member

Yes, it does look like the right solution. Can we cover it with tests though?

@Uzlopak
Copy link
Contributor Author

Uzlopak commented Jan 5, 2022

tbh I have no idea how to simulate an aborted upload.

@Uzlopak
Copy link
Contributor Author

Uzlopak commented Jan 5, 2022

Do you have any idea how to implement it?

@mccare
Copy link

mccare commented Jan 6, 2022

Looking through the tests, I would do it similar to the multipart-pause (

reqChunks.forEach(function (buf) {
)

Btw, would you need to clear the event listener on busboy after the file has been succesfully processed?

@mcollina
Copy link
Member

Any update on this one?

@kibertoad
Copy link
Member

I can try adding the test in a day or two if @Uzlopak doesn't do it first.

@Uzlopak
Copy link
Contributor Author

Uzlopak commented Jan 17, 2022

If you call destroy again, it is handled as a no-op. https://nodejs.org/api/stream.html#writabledestroyerror
So it is no problem to destroy it again.

Regarding the removal of the listener: Does it really matter? It either fails totally or it succeeds together. We dont pass an error object to destroy, so destroying the busboy-stream after we processed everything should not result in any additional errors. So if the FileStream got piped and processed it can be savely destroyed, imho.

@Uzlopak
Copy link
Contributor Author

Uzlopak commented Apr 28, 2022

So what is now with this PR?

@kibertoad kibertoad merged commit e41a643 into master Jun 9, 2022
@kibertoad kibertoad deleted the passthrough-unpipe-destroy-cancel-to-filestream branch June 9, 2022 19:43
Uzlopak added a commit that referenced this pull request Jan 7, 2023
Eomm pushed a commit that referenced this pull request Jan 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Canceled uploads are not propagated to the part.file FileReadStream
4 participants