Skip to content

Added progress capturing support for node.js environment;#4215

Closed
DigitalBrainJS wants to merge 2 commits intoaxios:mainfrom
DigitalBrainJS:feat/node-capture-progress
Closed

Added progress capturing support for node.js environment;#4215
DigitalBrainJS wants to merge 2 commits intoaxios:mainfrom
DigitalBrainJS:feat/node-capture-progress

Conversation

@DigitalBrainJS
Copy link
Copy Markdown
Collaborator

@DigitalBrainJS DigitalBrainJS commented Oct 24, 2021

This PR adds the ability to capture upload & download progress in node.js environment. All axios data types (stream, buffer, string, ArrayBuffer, Uint8Array) are supported as they are converted to a stream if progress capture is required.

  • To make this work http adapter was extensively refactored
  • Data conversion for non-stream data type fully relies on the Buffer.from method.
  • onUploadProgress & onDownloadProgress handlers are throttled by the limit of 10 calls per second. These handlers will receive the following data structure: {loaded: Number, total: Number, progress: Float}
  • Added Headers helper class to work with case insensitive headers;
  • maxBodyLength option of the default config object was explicitly set to 10MB;
    Update:
  • rolled back some refactoring changes to keep compatibility with recent merged PR's
  • added missed early exit if URL parsing fails

@DigitalBrainJS DigitalBrainJS marked this pull request as draft October 25, 2021 09:19
@DigitalBrainJS DigitalBrainJS force-pushed the feat/node-capture-progress branch 2 times, most recently from 70451f1 to 0fb24a6 Compare October 25, 2021 22:48
Added `Headers` helper class;
Refactored `http` adapter;
`maxBodyLength` was set explicitly to 10MB in the default config object;
@DigitalBrainJS DigitalBrainJS force-pushed the feat/node-capture-progress branch from 0fb24a6 to c8e4234 Compare October 25, 2021 22:51
@DigitalBrainJS DigitalBrainJS marked this pull request as ready for review October 25, 2021 22:51
…e-capture-progress;

Fixed missed early exit when URL parsing fails;
@jasonsaayman
Copy link
Copy Markdown
Member

@DigitalBrainJS this one seems relevant too, but I think as with the other pull request this probably needs some attention 😄, tag me when you are ready for me to look at it and merge

@DigitalBrainJS DigitalBrainJS marked this pull request as draft May 17, 2022 20:22
@jasonsaayman
Copy link
Copy Markdown
Member

@DigitalBrainJS please let me know if you will get this done soon, I would like to include it in v1?

@DigitalBrainJS
Copy link
Copy Markdown
Collaborator Author

DigitalBrainJS commented May 20, 2022

@jasonsaayman The PR refactoring is stuck because it depends on the new Headers interface, but its implementation brings significant breaking changes for users in the transformation chains as the headers become a map-like object rather than a hash object.
Here are two ways to choose from:

  • use the new Headers interface only internally, leave the headers in the transforms chain as is (plain object). This is a partial refactoring (this PR goes in this way, but...)
  • full refactoring. The user will have to use the header.set/get/has/delete methods.
    Benefits: The interface is case insensitive, can preserve the user's header name case to make headers more readable, supports false value to ask Axios not to add a header unless it's really necessary, and supports filtering/matching.

So I'm going to split this in two PR's - with Headers helper and progress capturing itself.

@jasonsaayman
Copy link
Copy Markdown
Member

I think for a v1 we should try not to go with the full refactor, we can do that asap with work on v2

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