Skip to content

feat: added destroying of part.file if the node request ends with an error event #319

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

Merged
merged 1 commit into from
Jan 20, 2022

Conversation

mccare
Copy link
Contributor

@mccare mccare commented Jan 17, 2022

If the client cancels the http request (e.g. just closes the socket),
the underlying file stream will not be closed with an error but
the request handling will hang.

The tests saves the stream of a file part into a Noop write stream
through pipeline. If the client cancels the request, the pipeline
call will not return. With the fix, we keep track of the file that
is currently being processed and in the case of an error event on the
node request, this file will be destroyed.

This addresses #315 and makes fastify/busboy#81 superflous.

Checklist

Sorry, something went wrong.

@mccare mccare force-pushed the close-file-on-http-request-error branch 2 times, most recently from b3292d0 to b2729a3 Compare January 18, 2022 00:26
…error event

If the client cancels the http request (e.g. just closes the socket),
the underlying file stream will not be closed with an error but
the request handling will hang.

The tests saves the stream of a file part into a Noop write stream
through pipeline. If the client cancels the request, the pipeline
call will not return. With the fix, we keep track of the file that
is currently being processed and in the case of an error event on the
node request, this file will be destroyed.
@mccare mccare force-pushed the close-file-on-http-request-error branch from b2729a3 to 0be4c4c Compare January 18, 2022 00:30
Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@mcollina mcollina merged commit 4e5b822 into fastify:master Jan 20, 2022
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.

None yet

3 participants