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

Implemented support for chunked transfer-encoding #397

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

danielgindi
Copy link

@coveralls
Copy link

coveralls commented Sep 3, 2018

Coverage Status

Coverage increased (+0.2%) to 98.206% when pulling a7bc1c3 on danielgindi:feature/chunked into 0170178 on form-data:master.

@danielgindi danielgindi force-pushed the feature/chunked branch 7 times, most recently from 88ba7dd to 25f6da9 Compare September 3, 2018 07:48
@danielgindi
Copy link
Author

AppVeyor fails due to permission issues

@BenjD90
Copy link

BenjD90 commented Oct 4, 2018

Is there any link with this issue (request/request#2499) ?

@alexindigo
Copy link
Member

@danielgindi Sorry, somehow I missed it. I'm loving it. Let me try to run it locally for sanity check and I'll check what's up with AppWeird.

Thanks a lot. It looks awesome.

@alexindigo
Copy link
Member

Seems like it's npm@2 issue. Let's update node version (both appveyor and travis) to 6, 8, 10.

@alexindigo
Copy link
Member

Looks like that whole "NaN as chunked" use case is missing
NaN therefore chunked

Do you mind to update tests to cover that?

@alexindigo
Copy link
Member

I think that and bumping node versions will bring us to the finish line.
Then we'll cut new major version and everybody will be happy. :)

Thank you.

@danielgindi
Copy link
Author

No problem, I found out that that uncovered line will indeed never happen in normal conditions.
It could only happen if a "familiar" (readable file, http request...) stream is supplied and then its properties will be changed just before calling getLength.
Since the library is not responsible for handling such race conditions, and this would be an unrecoverable situation anyway - I removed that redundant line.

@danielgindi
Copy link
Author

And I'll let you bump the version :-)

@danielgindi
Copy link
Author

@alexindigo any news? currently having to do hacks in code to support this...

@naugtur
Copy link

naugtur commented Mar 12, 2019

For anyone who ends up here from issues with multipat where you wish to set encoding to chunked.

To prevent request from setting content-length on the request without any changes to it and this library you must set
knownLength: NaN

That will explicitly propagate through length counting and cause resulting total length to be NaN thus stopping request from setting content-length.

If properly documented this might be considered a feature ;)

@danielgindi
Copy link
Author

@naugtur This is a very nice workaround :-)

@Exegetech
Copy link

@naugtur you just saved me....

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

6 participants