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

uploadProgress for non-stream bodies #1735

Closed
2 tasks done
Vivalize opened this issue May 31, 2021 · 7 comments
Closed
2 tasks done

uploadProgress for non-stream bodies #1735

Vivalize opened this issue May 31, 2021 · 7 comments
Labels
documentation The issue will improve the docs wontfix The issue cannot be fixed on Got side

Comments

@Vivalize
Copy link

Describe the bug

I'm sending a POST request to a server with a large string as the body, and would like to get granular progress events as it sends. However, the on('uploadProgress') listener only seems to fire at the start and end of the request, and never mid-transit. I got the impression from #322 that a workaround was implemented to make this work, but I can't seem to achieve this. If something else is needed from my code to make it work, it would be nice to see more clarification in the documentation.

  • Node.js version: v14.17.0
  • OS & version: macOS Big Sur 11.2.3

Actual behavior

{ percent: 0, transferred: 0, total: 10000000 }
{ percent: 1, transferred: 10000000, total: 10000000 }

Expected behavior

{ percent: 0, transferred: 0, total: 10000000 }
{ percent: 0.1, transferred: 1000000, total: 10000000 }
{ percent: 0.2, transferred: 2000000, total: 10000000 }
...
{ percent: 0.8, transferred: 8000000, total: 10000000 }
{ percent: 0.9, transferred: 9000000, total: 10000000 }
{ percent: 1, transferred: 10000000, total: 10000000 }

Code to reproduce

const got = require('got')

// This is a barebones http server to reproduce realistic round-trip request times
const server = 'https://got-progress-test.herokuapp.com/'

// Generate a random string to simulate a large request body size
const randomString = Array.from({length: 10000000}, () => String.fromCharCode(Math.floor(Math.random() * 57 + 65))).join('')

// Send the string as the body of a post request
got.post(server, { body: randomString })
    .on('uploadProgress', progress => console.log(progress))

Checklist

  • I have read the documentation.
  • I have tried my code with the latest version of Node.js and Got.
@szmarczak szmarczak added bug Something does not work as it should regression Something does not work anymore labels May 31, 2021
@szmarczak
Copy link
Collaborator

Indeed, this regressed in Got 11. A workaround is to set body to Readable.from(string)

@szmarczak
Copy link
Collaborator

Unfortunately Readable.from(string) is the only solution. Node.js does not support upload progress natively.

@szmarczak
Copy link
Collaborator

This will result in duplicated data though. If you send a 1MB Buffer then Got will store it twice - in the stream and in the options so you can inspect it for debugging.

@szmarczak
Copy link
Collaborator

szmarczak commented May 31, 2021

@sindresorhus Maybe let's enable progress callbacks for Stream API and stream bodies only?

HTTP/1 streams are not real streams, so we cannot read writableState. We can read from the socket directly, but having in mind that HTTP/2 is so close to be the main transport and HTTP/3 is on the horizon I'd prefer if we didn't read from the socket.

@szmarczak
Copy link
Collaborator

If undici supported this then it would be great: nodejs/undici#820

@szmarczak szmarczak changed the title .on('uploadProgress') only fires twice (at beginning and end) of POST request with a string body uploadProgress for non-stream bodies May 31, 2021
@sindresorhus
Copy link
Owner

I agree.

@szmarczak
Copy link
Collaborator

szmarczak commented Jul 1, 2021

While undici probably will support progress callbacks, it won't automatically split body into chunks.

You have to do it manually and it will result in duplicated data.

@szmarczak szmarczak added documentation The issue will improve the docs wontfix The issue cannot be fixed on Got side and removed regression Something does not work anymore bug Something does not work as it should labels Jul 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation The issue will improve the docs wontfix The issue cannot be fixed on Got side
Projects
None yet
Development

No branches or pull requests

3 participants