orangewise/s3-zip

Process crash if inner stream throws an error

mohuk opened this issue · 0 comments

mohuk commented

Package version: 1.0.1
Node version: 8.9.4

Facing an error when we try to download a file from S3 which doesn't exist in the bucket. A simple use case is when a user is given a choice to download the documents and they somehow enter an S3 key which doesn't exist in the bucket.

Current behaviour:
Entering an incorrect key crashes the entire process with the following trace.

 NoSuchKey: The specified key does not exist.
     at Request.extractError (/app/node_modules/aws-sdk/lib/services/s3.js:577:35)
     at Request.callListeners (/app/node_modules/aws-sdk/lib/sequential_executor.js:105:20)
     at Request.emit (/app/node_modules/aws-sdk/lib/sequential_executor.js:77:10)
     at Request.emit (/app/node_modules/aws-sdk/lib/request.js:683:14)
     at Request.transition (/app/node_modules/aws-sdk/lib/request.js:22:10)
     at AcceptorStateMachine.runTo (/app/node_modules/aws-sdk/lib/state_machine.js:14:12)
     at /app/node_modules/aws-sdk/lib/state_machine.js:26:10
     at Request.<anonymous> (/app/node_modules/aws-sdk/lib/request.js:38:9)
     at Request.<anonymous> (/app/node_modules/aws-sdk/lib/request.js:685:12)
     at Request.callListeners (/app/node_modules/aws-sdk/lib/sequential_executor.js:115:18)
     at Request.emit (/app/node_modules/aws-sdk/lib/sequential_executor.js:77:10)
     at Request.emit (/app/node_modules/aws-sdk/lib/request.js:683:14)
     at Request.transition (/app/node_modules/aws-sdk/lib/request.js:22:10)
     at AcceptorStateMachine.runTo (/app/node_modules/aws-sdk/lib/state_machine.js:14:12)
     at /app/node_modules/aws-sdk/lib/state_machine.js:26:10
     at Request.<anonymous> (/app/node_modules/aws-sdk/lib/request.js:38:9)
     at Request.<anonymous> (/app/node_modules/aws-sdk/lib/request.js:685:12)
     at Request.callListeners (/app/node_modules/aws-sdk/lib/sequential_executor.js:115:18)
     at callNextListener (/app/node_modules/aws-sdk/lib/sequential_executor.js:95:12)
     at IncomingMessage.onEnd (/app/node_modules/aws-sdk/lib/event_listeners.js:269:13)
     at emitNone (events.js:111:20)
     at IncomingMessage.emit (events.js:208:7)
 [nodemon] app crashed - waiting for file changes before starting...

Suggested fix:
Internal stream which fetches the data from S3 via aws-sdk does not have error events attached to it. Also, try/catch/throw does not work for streams, they only understand events e.g. .on('data). Attaching .on('error') on the internal file stream and emitting the error on the outer stream should resolve the issue.

PR coming :)