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

Fix readableStream unexpected end while waiting response #109

Merged
merged 1 commit into from May 2, 2013

Conversation

Projects
None yet
3 participants
@Rafe
Contributor

Rafe commented May 1, 2013

Hello,

recently while using createReadStream() to pipe response in AWS.S3, under Node v0.10.1
I found the stream will hangout unexpected and only return partial result:

var buffers = [];
var s3 = new AWS.S3();
var stream = s3.getObject(params).createReadStream();
stream.on('readable', function(){
  var data;
  while (data = stream.read()){
    buffers.push(data);
  };
});
stream.on('end', function(){
  fs.writeFileSync('./test.jpg', Buffer.concat(buffers));
}); // result in partial image

After checking the source of createReadStream(),
I found the stream._read method is missing a push method when httpStream.read() return false values, which result to the unexpected end:

stream._read = function() {
  var data;
  /*jshint boss:true*/
  while (data = httpStream.read()) {
    stream.push(data);
  }
};

What happened here, is because when the read method on stream is called, it will set a reading flag to true, until the stream.push is called, it will block next read call until the previous read finished. Therefore when httpStream is still waiting for response, the readStream will block itself because of the missing push call.

Further details is in the source of issac/readable-stream
https://github.com/isaacs/readable-stream/blob/master/lib/_stream_readable.js#L286

So the pipe is also working now under 0.10.x,

var out = fs.createWriteStream('./nyancat.jpg');
s3.getObject(params).createReadStream().pipe(out);

Let me know is this solves the problem, also I made some hack on helper to emulate empty read call. Probably will have better way to emulate it.

lsegal added a commit that referenced this pull request May 2, 2013

Merge pull request #109 from Rafe/patch/stream2
Fix readableStream unexpected end while waiting response

@lsegal lsegal merged commit fcae7f8 into aws:master May 2, 2013

1 check passed

default The Travis build passed
Details
@lsegal

This comment has been minimized.

Contributor

lsegal commented May 2, 2013

This looks like the correct interpretation to me. Thanks for fixing this up with a test!

@Rafe Rafe deleted the Rafe:patch/stream2 branch May 2, 2013

@lancecarlson

This comment has been minimized.

lancecarlson commented May 3, 2013

I just ran into this issue, and this seemed to fix it. Can we make another bump in the version? Seems pretty critical that this behave appropriately.

@lsegal

This comment has been minimized.

Contributor

lsegal commented May 3, 2013

Hi @lancecarlson, you can install the npm from github in the meantime:

npm install git://github.com/aws/aws-js-sdk
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment