Merge pull request #76 from overleaf/spd-general-errors

Handle AccessDenied and stream-premature-close errors

services/filestore/app/js/FileController.js

@@ -62,8 +62,16 @@ function getFile(req, res, next) {
 
     logger.log({ key, bucket, format, style }, 'sending file to response')
 
-    // pass 'next' as a callback to 'pipeline' to receive any errors
+    pipeline(fileStream, res, err => {
-    pipeline(fileStream, res, next)
+      if (err && err.code !== 'ERR_STREAM_PREMATURE_CLOSE') {
+        logger.err(
+          new Errors.ReadError({
+            message: 'error transferring stream',
+            info: { bucket, key, format, style }
+          }).withCause(err)
+        )
+      }
+    })
   })
 }
 

services/filestore/app/js/S3PersistorManager.js

@@ -236,7 +236,9 @@ async function directorySize(bucketName, key) {
 }
 
 function _wrapError(error, message, params, ErrorType) {
-  if (['NoSuchKey', 'NotFound', 'ENOENT'].includes(error.code)) {
+  if (
+    ['NoSuchKey', 'NotFound', 'AccessDenied', 'ENOENT'].includes(error.code)
+  ) {
     return new NotFoundError({
       message: 'no such file',
       info: params

services/filestore/test/unit/js/S3PersistorManagerTests.js

@@ -37,6 +37,7 @@ describe('S3PersistorManagerTests', function() {
     S3Client,
     S3ReadStream,
     S3NotFoundError,
+    S3AccessDeniedError,
     FileNotFoundError,
     EmptyPromise,
     settings
@@ -84,6 +85,9 @@ describe('S3PersistorManagerTests', function() {
     S3NotFoundError = new Error('not found')
     S3NotFoundError.code = 'NoSuchKey'
 
+    S3AccessDeniedError = new Error('access denied')
+    S3AccessDeniedError.code = 'AccessDenied'
+
     S3ReadStream = {
       on: sinon.stub(),
       pipe: sinon.stub().returns('s3Stream'),
@@ -291,6 +295,36 @@ describe('S3PersistorManagerTests', function() {
       })
     })
 
+    describe('when access to the file is denied', function() {
+      let error, stream
+
+      beforeEach(async function() {
+        S3ReadStream.on = sinon.stub()
+        S3ReadStream.on.withArgs('error').yields(S3AccessDeniedError)
+        try {
+          stream = await S3PersistorManager.promises.getFileStream(bucket, key)
+        } catch (err) {
+          error = err
+        }
+      })
+
+      it('does not return a stream', function() {
+        expect(stream).not.to.exist
+      })
+
+      it('throws a NotFoundError', function() {
+        expect(error).to.be.an.instanceOf(Errors.NotFoundError)
+      })
+
+      it('wraps the error from S3', function() {
+        expect(error.cause).to.equal(S3AccessDeniedError)
+      })
+
+      it('stores the bucket and key in the error', function() {
+        expect(error.info).to.deep.equal({ Bucket: bucket, Key: key })
+      })
+    })
+
     describe('when S3 encounters an unkown error', function() {
       let error, stream