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

Cannot calculate proper length in synchronous way. (node-fetch + form-data) #102

Closed
mahnunchik opened this issue Apr 12, 2016 · 16 comments
Closed

Comments

@mahnunchik
Copy link

Error: Cannot calculate proper length in synchronous way.
      at FormData.getLengthSync (node_modules/form-data/lib/form_data.js:245:17)
      at node_modules/node-fetch/index.js:96:48
      at new Fetch (node_modules/node-fetch/index.js:49:9)
      at Fetch (node_modules/node-fetch/index.js:37:10)

This line causes the error: https://github.com/bitinn/node-fetch/blob/master/index.js#L96

@bitinn
Copy link
Collaborator

bitinn commented Apr 12, 2016

This is a hack to support form-data module, so you don't have to set content-length header manually. You might want to figure out why form-data throws this error in the first place (probably related to your body data type).

@mahnunchik
Copy link
Author

@bitinn

I've tried to send following body:

form.append('my_file', fs.createReadStream('/foo/bar.jpg'));

@bitinn
Copy link
Collaborator

bitinn commented Apr 12, 2016

Due to the nature of callback, we don't currently use getLength to determine form-data's content-length, but you can calculate the content-length yourself (either with FormData.getLength or count the stream byte itself) then pass it onto node-fetch's headers option manually, this will skip the hack as content-length is provided already.

@mcfedr
Copy link

mcfedr commented Apr 14, 2016

I have worked around this bug for the moment by removing the getLengthSync method before passing it to node-fetch

form.getLengthSync = null;

Some servers don't like it not having a content-length header, but its certainly not required by the spec and many servers will work fine without it.

@bitinn
Copy link
Collaborator

bitinn commented Apr 14, 2016

@mcfedr please don't do this, why can't you call getLength to figure out content-length?

UPDATE: see my reply here #106 (comment)

@bitinn
Copy link
Collaborator

bitinn commented Apr 14, 2016

Alternative, don't use form-data, because you are just sending files right? Only use form-data to manage your custom headers, let node-fetch handle the body as native stream instead of form-data instance.

@mcfedr
Copy link

mcfedr commented Apr 14, 2016

For local files its possible to use fs.stat and set knownLength when adding to form-data

I am sending another stream, and I dont know the length of the stream. Here are example of how things are broken, these are bugs in both node-fetch and form-data

https://gist.github.com/mcfedr/0a6fbaeaf0cfaddbeadfe145c002c8e5

I think in the 'http' example node-fetch should ignore the correctly thrown error and be ok with not setting a content-length

In the 'fetch' example form-data needs to throw the error because it doesnt know its length

@bitinn
Copy link
Collaborator

bitinn commented May 6, 2016

Quick summary: unfortunately form-data doesn't have an API for checking whether we can call getLengthSync yet, so we have 3 options:

  1. to do it correctly and manually, end user can do the calculation with form.getLength callback and overwrite our default behavior with custom content-length
  2. to not add content-length at all, use the form.getLengthSync = null hack mcfedr mentioned to prevent node-fetch from calling getLengthSync
  3. we might add a formdata._lengthRetrievers.length > 0 check to skip calling getLengthSync automagically, as discussed in Is there a way we can explicit check if we shouldn't call getLengthSync? form-data/form-data#196

@bitinn
Copy link
Collaborator

bitinn commented May 19, 2016

No response from form-data dev, I am going with option 3.

@havenchyk
Copy link

@bitinn @mcfedr do you have working workaround right now?

@bitinn
Copy link
Collaborator

bitinn commented May 24, 2016

@havenchyk I will update this issue when release, but you should read my reply above #102 (comment)

@havenchyk
Copy link

@bitinn thanks for the quick response. I actually read it and understand that I need to skip the check somehow.

My problem is that everything worked fine for a year and now it's broken. I'm trying to figure out, is it a problem of FormData(maybe from last releases) or of node-fetch.

@mcfedr
Copy link

mcfedr commented May 24, 2016

@havenchyk my work around works, #102 (comment)

@bitinn
Copy link
Collaborator

bitinn commented May 24, 2016

@havenchyk I need to see the error to conclusively say you are facing the same problem. But form-data has been doing this for quite some time, so unless you changed your data source to a stream, I don't see why it would suddenly break (unless you upgrade from a very old node-fetch which doesn't use hack to handle form-data previously)

@havenchyk
Copy link

@bitinn it's definitely the same, workaround from @mcfedr solved my issue. Can I help you to make release faster?

@bitinn
Copy link
Collaborator

bitinn commented May 25, 2016

workaround has been added and released as v1.5.3

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

No branches or pull requests

4 participants