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

s3.upload is storing empty data when Body is a request module response stream. #1277

Closed
aniltallam opened this issue Dec 20, 2016 · 5 comments
Closed
Labels

Comments

@aniltallam
Copy link

@aniltallam aniltallam commented Dec 20, 2016

s3.upload is storing empty data when Body is a request module response stream.

code:

request(getOptions).on('response', function (response) {
    var destinationParams = { Key: 'key1', Body: response}
    s3Client.upload(destinationParams, function (err, data) {
            //...
    })
})
@aniltallam

This comment has been minimized.

Copy link
Author

@aniltallam aniltallam commented Dec 20, 2016

Investigation:
Actually request module is registering to 'data' event on response streams. Aws s3 upload is listening to readable event and trying to get data using response.read() which is returning null because data was already drained using 'data' event handler by request module. so it empty data is sent to aws server.

So currently we are using this workaround:

request(getOptions).on('response', function (response) {
    var readableStream = new PassThrough()
    response.pipe(readableStream)
    var destinationParams = { Key: 'key1', Body: readableStream}
    s3Client.upload(destinationParams, function (err, data) {
            //...
    })
})

It would be clean and idle if s3client handles this scenario like other modules like fs (writeFile), process.stdout which are able to fetch data from request->response stream.
eg:

fs.writeFile(request_response)

(or)

request_response.pipe(process.stdout)
@jeskew

This comment has been minimized.

Copy link
Contributor

@jeskew jeskew commented Jan 6, 2017

@aniltallam Have you tried using the createReadStream method on the request to create a readable stream rather than attaching a listener to the 'response' event?

@aniltallam

This comment has been minimized.

Copy link
Author

@aniltallam aniltallam commented Jan 9, 2017

@jeskew here request method is from request npm module (var request = require('request'))

@jeskew jeskew added the duplicate label Jan 9, 2017
@jeskew

This comment has been minimized.

Copy link
Contributor

@jeskew jeskew commented Jan 9, 2017

Thanks for clarifying! There's some discussion in #686 about request binding to the 'data' event and thereby changing the underlying stream from paused mode to flowing mode, which means that there could be data lost between when the request data starts arriving and when the SDK begins uploading to S3. I'm closing this issue as a duplicate of the original one, but please feel free to renew the discussion on that issue.

In the meantime, have you tried using request's .pipe() method instead? They have an example of how to do so in their README.

@jeskew jeskew closed this Jan 9, 2017
@lock

This comment has been minimized.

Copy link

@lock lock bot commented Sep 29, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs and link to relevant comments in this thread.

@lock lock bot locked as resolved and limited conversation to collaborators Sep 29, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.