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

fixed readAllBodyWithStreamToFile function #123

Merged
merged 6 commits into from
Jul 22, 2024

Conversation

tigersoft6084
Copy link
Contributor

@tigersoft6084 tigersoft6084 commented Jul 19, 2024

I was using tls-client in conjunction with Node.js and encountered an issue while handling large file downloads. Specifically, I tried to set streamOutputPath to save the response body directly to a file. However, tls-client attempted to send the entire response as a JSON object, causing it to hang. Additionally, when using the withDebug option, it printed the entire response body, which was very inefficient.

To address these issues, I made the following updates to the code:

  1. Empty Response Body with streamOutputPath: When streamOutputPath is set, the code now returns an empty response body instead of attempting to convert the entire response to JSON.
  2. EOF Error Handling: Fixed a bug that prevented the last body snippet from being processed correctly due to an EOF error.

These changes ensure that large file downloads are handled efficiently and that the response body is managed appropriately when using streamOutputPath.

Copy link
Owner

@bogdanfinn bogdanfinn left a comment

Choose a reason for hiding this comment

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

Thank you for your contribution. i only have two minor comments on your changeset :)

cffi_dist/go.mod Outdated Show resolved Hide resolved
cffi_dist/build.sh Outdated Show resolved Hide resolved
@tigersoft6084 tigersoft6084 requested a review from bogdanfinn July 21, 2024 14:00
@bogdanfinn bogdanfinn merged commit 2a633b7 into bogdanfinn:master Jul 22, 2024
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.

2 participants