Rethrown error when streaming

[BorePlusPlus]

Hello,

Recently I noticed that my streaming download code is "blowing up" the process when trying to download non-existing object (my guess is it's probably same for all error conditions). The stripped down use case looks like this.

'use strict';

var AWS = require('aws-sdk');
var fs = require('fs');

AWS.config.update({
    accessKeyId: 'foo',
    secretAccessKey: 'bar'
});

var s3Client = new AWS.S3();
var devNull = fs.createWriteStream('/dev/null');

function getStream(s3id) {
    var s3DownloadParams = {
        Bucket: 'baz',
        Key: s3id
    };

    return s3Client.getObject(s3DownloadParams)
        .on('error', function () { console.log(':('); })
        .on('success', function () { console.log(':)'); })
        .createReadStream();
}

getStream('non-existing').pipe(devNull);

Pre 1e277ec this just printed the sad face :(, but after that commit the process is killed by an unhandled error, a bit like so:

:(

/home/bore/projects/playground/aws-sdk-js/lib/sequential_executor.js:226
        throw err;
              ^
NoSuchKey: The specified key does not exist.
    at Request.extractError (/home/bore/projects/playground/aws-sdk-js/lib/services/s3.js:306:35)
    at Request.callListeners (/home/bore/projects/playground/aws-sdk-js/lib/sequential_executor.js:102:18)
    at Request.emit (/home/bore/projects/playground/aws-sdk-js/lib/sequential_executor.js:79:10)
    at Request.emit (/home/bore/projects/playground/aws-sdk-js/lib/request.js:595:14)
    at Request.transition (/home/bore/projects/playground/aws-sdk-js/lib/request.js:20:12)
    at AcceptorStateMachine.runTo (/home/bore/projects/playground/aws-sdk-js/lib/state_machine.js:14:12)
    at /home/bore/projects/playground/aws-sdk-js/lib/state_machine.js:26:10
    at Request.<anonymous> (/home/bore/projects/playground/aws-sdk-js/lib/request.js:21:9)
    at Request.<anonymous> (/home/bore/projects/playground/aws-sdk-js/lib/request.js:597:12)
    at Request.callListeners (/home/bore/projects/playground/aws-sdk-js/lib/sequential_executor.js:106:18)

Could you please spare some time to look into this (or suggest alternative implementation)?

Cheers,
Dali

[lsegal]

It looks like this only happens with .createReadStream(), so it should be easier to narrow down. Taking a look.

[lsegal]

So the rethrown error is not actually coming from the SDK, but rather, the stream object. Handling the 'error' event on the stream fixes it for me:

getStream('non-existing').
  on('error', function() { console.log('ERR!'); }).
  pipe(devNull);

Output:

$ node index
:(
ERR!

That is ideally where you want to do error handling on streams, as there could be other errors coming from the stream itself. The SDK has always piped all errors from the error event to the stream's error event so there would be a single unified event for all errors-- I'm not entirely sure why that didn't cause it to be thrown before. Perhaps the SDK wasn't correctly piping these specific errors before?

[BorePlusPlus]

Thanks @lsegal, that works for me.

Is there some documentation mentioning this that I have missed?

[lsegal]

@BorePlusPlus because the SDK simply returns a regular stream object, the behavior for createReadStream() should be equivalent to Node.js's own stream module. I just took a look and it doesn't seem as though Node's docs really call out the error logic for streams besides very basic documentation for the 'error' event. They don't really explain that you need to hook up an event or it will throw:

fs.createReadStream('invalid-file'); // throws Error: ENOENT, open 'invalid-file'

fs.createReadStream('invalid-file').
  on('error', function() { console.log(':(') }); // prints ":("

Marking this as closed since it seems like the SDK is (now) behaving as expected. Feel free to open another issue if you have any more questions or problems!

[lock]

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.