s3: ensure BoxError return

src/backups.js

@@ -565,16 +565,16 @@ function downloadDir(backupConfig, backupFilePath, dataLayout, progressCallback,
 
     debug(`downloadDir: ${backupFilePath} to ${dataLayout.toString()}`);
 
-    function downloadFile(entry, callback) {
+    function downloadFile(entry, done) {
         let relativePath = path.relative(backupFilePath, entry.fullPath);
         if (backupConfig.key) {
             relativePath = decryptFilePath(relativePath, backupConfig.key);
-            if (!relativePath) return callback(new BoxError(BoxError.BAD_STATE, 'Unable to decrypt file'));
+            if (!relativePath) return done(new BoxError(BoxError.BAD_STATE, 'Unable to decrypt file'));
         }
         const destFilePath = dataLayout.toLocalPath('./' + relativePath);
 
         mkdirp(path.dirname(destFilePath), function (error) {
-            if (error) return callback(new BoxError(BoxError.FS_ERROR, error.message));
+            if (error) return done(new BoxError(BoxError.FS_ERROR, error.message));
 
             async.retry({ times: 5, interval: 20000 }, function (retryCallback) {
                 let destStream = createWriteStream(destFilePath, backupConfig.key || null);
@@ -597,21 +597,21 @@ function downloadDir(backupConfig, backupFilePath, dataLayout, progressCallback,
                     if (error) return closeAndRetry(error);
 
                     sourceStream.on('error', closeAndRetry);
-                    destStream.on('error', closeAndRetry);
+                    destStream.on('error', closeAndRetry); // already emits BoxError
 
                     progressCallback({ message: `Downloading ${entry.fullPath} to ${destFilePath}` });
 
                     sourceStream.pipe(destStream, { end: true }).on('finish', closeAndRetry);
                 });
-            }, callback);
+            }, done);
         });
     }
 
-    api(backupConfig.provider).listDir(backupConfig, backupFilePath, 1000, function (entries, done) {
+    api(backupConfig.provider).listDir(backupConfig, backupFilePath, 1000, function (entries, iteratorDone) {
         // https://www.digitalocean.com/community/questions/rate-limiting-on-spaces?answer=40441
         const concurrency = backupConfig.downloadConcurrency || (backupConfig.provider === 's3' ? 30 : 10);
 
-        async.eachLimit(entries, concurrency, downloadFile, done);
+        async.eachLimit(entries, concurrency, downloadFile, iteratorDone);
     }, callback);
 }
 
@@ -623,7 +623,7 @@ function download(backupConfig, backupId, format, dataLayout, progressCallback,
     assert.strictEqual(typeof progressCallback, 'function');
     assert.strictEqual(typeof callback, 'function');
 
-    debug(`download - Downloading ${backupId} of format ${format} to ${dataLayout.toString()}`);
+    debug(`download: Downloading ${backupId} of format ${format} to ${dataLayout.toString()}`);
 
     const backupFilePath = getBackupFilePath(backupConfig, backupId, format);
 

src/storage/s3.js

@@ -165,7 +165,7 @@ function listDir(apiConfig, dir, batchSize, iteratorCallback, callback) {
 
         async.whilst(() => !done, function listAndDownload(whilstCallback) {
             s3.listObjects(listParams, function (error, listData) {
-                if (error) return whilstCallback(error);
+                if (error) return whilstCallback(new BoxError(BoxError.EXTERNAL_ERROR, error.message || error.code));
 
                 if (listData.Contents.length === 0) { done = true; return whilstCallback(); }
 
@@ -280,7 +280,7 @@ function copy(apiConfig, oldFilePath, newFilePath) {
 
                         events.emit('progress', `Uploaded part ${partCopyParams.PartNumber} - Etag: ${result.CopyPartResult.ETag}`);
 
-                        if (!result.CopyPartResult.ETag) return done(new BoxError(BoxError.EXTERNAL_ERROR, 'Multi-part copy is broken or not implemented by the S3 storage provider'));
+                        if (!result.CopyPartResult.ETag) return done(new Error('Multi-part copy is broken or not implemented by the S3 storage provider'));
 
                         uploadedParts.push({ ETag: result.CopyPartResult.ETag, PartNumber: partNumber });
 
@@ -349,9 +349,9 @@ function remove(apiConfig, filename, callback) {
 
         // deleteObjects does not return error if key is not found
         s3.deleteObjects(deleteParams, function (error) {
-            if (error) debug(`remove: Unable to remove ${deleteParams.Key}. error: ${error.message}`);
+            if (error) return callback(new BoxError(BoxError.EXTERNAL_ERROR, `Unable to remove ${deleteParams.Key}. error: ${error.message || error.code}`)); // DO sets 'code'
 
-            callback(error);
+            callback(null);
         });
     });
 }
@@ -386,9 +386,12 @@ function removeDir(apiConfig, pathPrefix) {
 
                 // deleteObjects does not return error if key is not found
                 s3.deleteObjects(deleteParams, function (error /*, deleteData */) {
-                    if (error) events.emit('progress', `Unable to remove ${deleteParams.Key} ${error.message}`);
+                    if (error) {
+                        events.emit('progress', `Unable to remove ${deleteParams.Key} ${error.message || error.code}`);
+                        return iteratorCallback(new BoxError(BoxError.EXTERNAL_ERROR, `Unable to remove ${deleteParams.Key}. error: ${error.message || error.code}`)); // DO sets 'code'
+                    }
 
-                    iteratorCallback(error);
+                    iteratorCallback(null);
                 });
             }, done);
         }, function (error) {