diff --git a/libraries/object-persistor/src/S3Persistor.js b/libraries/object-persistor/src/S3Persistor.js index 110414d560..1a8c326be4 100644 --- a/libraries/object-persistor/src/S3Persistor.js +++ b/libraries/object-persistor/src/S3Persistor.js @@ -184,7 +184,7 @@ class S3Persistor extends AbstractPersistor { case 404: // NoSuchKey return reject(new NotFoundError('not found')) default: - return reject(new Error('non success status: ' + statusCode)) + // handled by stream.on('error') handler below } }) // The AWS SDK is forwarding any errors from the request to the stream. diff --git a/libraries/object-persistor/test/unit/S3PersistorTests.js b/libraries/object-persistor/test/unit/S3PersistorTests.js index cc785034a2..37c21c0050 100644 --- a/libraries/object-persistor/test/unit/S3PersistorTests.js +++ b/libraries/object-persistor/test/unit/S3PersistorTests.js @@ -91,6 +91,14 @@ describe('S3PersistorTests', function () { createReadStream() { setTimeout(() => { + if (this.notFoundSSEC) { + // special case for AWS S3: 404 NoSuchKey wrapped in a 400. A single request received a single response, and multiple httpHeaders events are triggered. Don't ask. + this.emit('httpHeaders', 400) + this.emit('httpHeaders', 404) + ReadStream.emit('error', S3NotFoundError) + return + } + if (this.err) return ReadStream.emit('error', this.err) this.emit('httpHeaders', this.statusCode) if (this.statusCode === 403) { @@ -344,6 +352,34 @@ describe('S3PersistorTests', function () { }) }) + describe("when the file doesn't exist -- SSEC", function () { + let error, stream + + beforeEach(async function () { + S3GetObjectRequest.notFoundSSEC = 404 + try { + stream = await S3Persistor.getObjectStream(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', function () { + expect(error.cause).to.exist + }) + + it('stores the bucket and key in the error', function () { + expect(error.info).to.include({ bucketName: bucket, key }) + }) + }) + describe('when access to the file is denied', function () { let error, stream diff --git a/services/filestore/test/acceptance/js/FilestoreTests.js b/services/filestore/test/acceptance/js/FilestoreTests.js index 276471cb1a..bdab787393 100644 --- a/services/filestore/test/acceptance/js/FilestoreTests.js +++ b/services/filestore/test/acceptance/js/FilestoreTests.js @@ -467,6 +467,7 @@ describe('Filestore', function () { }) describe('with a large file', function () { + this.timeout(1000 * 20) let fileId, fileUrl, largeFileContent, error beforeEach('upload large file', async function () { @@ -1221,6 +1222,8 @@ describe('Filestore', function () { for (const { name, prev, cur, fail } of stages) { describe(name, function () { + this.timeout(1000 * 30) + it('can read old writes', async function () { await checkWrites(fileKey1, prev, [prev, cur], fail) await checkWrites(fileKey2, prev, [prev, cur], fail) // check again after access