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

companion: add calculated content-length for formdata and body cases #2466

Merged
merged 4 commits into from
Aug 28, 2020

Conversation

mejiaej
Copy link
Contributor

@mejiaej mejiaej commented Aug 14, 2020

Hi, as described in this PR content-length is an expected header when uploading to s3. In this PR I'm adding context-length calculation for the formData scenario. Had to factor out the http request since form.getLength from form-data expects a callback.

@ifedapoolarewaju ifedapoolarewaju self-requested a review August 17, 2020 14:24
@ifedapoolarewaju
Copy link
Contributor

Thank you for the PR! I'll review this week

@mejiaej mejiaej force-pushed the master branch 2 times, most recently from 933710e to 4cef783 Compare August 17, 2020 23:04
Copy link
Contributor

@ifedapoolarewaju ifedapoolarewaju 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 the work! 🙏

I dropped some comments/suggestions

packages/@uppy/companion/src/server/Uploader.js Outdated Show resolved Hide resolved
packages/@uppy/companion/src/server/Uploader.js Outdated Show resolved Hide resolved
packages/@uppy/companion/src/server/Uploader.js Outdated Show resolved Hide resolved
statusText: response.statusMessage,
headers
}
_handleUploadMultipart (error, response, body, bytesUploaded) {
Copy link
Contributor

Choose a reason for hiding this comment

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

should we rename this to _onMultipartComplete or something similar?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Totally agree, I think it is more descriptive.

@ifedapoolarewaju ifedapoolarewaju merged commit 886f059 into transloadit:master Aug 28, 2020
@ifedapoolarewaju
Copy link
Contributor

thank you for the work here! 🎉

@mejiaej
Copy link
Contributor Author

mejiaej commented Aug 28, 2020

@ifedapoolarewaju Thanks for the review and feedback.

HeavenFox pushed a commit to docsend/uppy that referenced this pull request Jun 27, 2023
…loadit#2466)

* companion: add calculated content-length for formdata and body cases

* companion: append fieldname and emit error in form data

Co-authored-by: Ifedapo .A. Olarewaju <ifedapoolarewaju@gmail.com>

* companion: change _handleUploadMultipart to _onMultipartComplete

* companion: use async fs.stat

Co-authored-by: Enrique Jhonatan Mejia Venegas <enrique.mejia@LIM-WS1807.local>
Co-authored-by: Ifedapo .A. Olarewaju <ifedapoolarewaju@gmail.com>
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