Ersun Warncke 6 лет назад
Родитель
Сommit
0e4be81406

+ 12 - 32
services/filestore/app/js/Errors.js

@@ -1,40 +1,20 @@
 const OError = require('@overleaf/o-error')
 
-// Error class for legacy errors so they inherit OError while staying
-// backward-compatible (can be instantiated with string as argument instead
-// of object)
-class BackwardCompatibleError extends OError {
-  constructor(messageOrOptions) {
-    let options
-    if (typeof messageOrOptions === 'string') {
-      options = { message: messageOrOptions }
-    } else if (!messageOrOptions) {
-      options = {}
-    } else {
-      options = messageOrOptions
-    }
-    super(options)
-  }
-}
-
-class NotFoundError extends BackwardCompatibleError {}
-class WriteError extends BackwardCompatibleError {}
-class ReadError extends BackwardCompatibleError {}
-class HealthCheckError extends BackwardCompatibleError {}
-class ConversionsDisabledError extends BackwardCompatibleError {}
-class ConversionError extends BackwardCompatibleError {}
-class SettingsError extends BackwardCompatibleError {}
-class TimeoutError extends BackwardCompatibleError {}
-class InvalidParametersError extends BackwardCompatibleError {}
+class NotFoundError extends OError {}
+class WriteError extends OError {}
+class ReadError extends OError {}
+class HealthCheckError extends OError {}
+class ConversionsDisabledError extends OError {}
+class ConversionError extends OError {}
+class SettingsError extends OError {}
+class TimeoutError extends OError {}
+class InvalidParametersError extends OError {}
 
 class FailedCommandError extends OError {
   constructor(command, code, stdout, stderr) {
-    super({
-      message: 'command failed with error exit code',
-      info: {
-        command,
-        code
-      }
+    super('command failed with error exit code', {
+      command,
+      code
     })
     this.stdout = stdout
     this.stderr = stderr

+ 8 - 6
services/filestore/app/js/FSPersistor.js

@@ -46,9 +46,11 @@ async function sendStream(location, target, sourceStream, sourceMd5) {
     const destMd5 = await getFileMd5Hash(location, target)
     if (sourceMd5 !== destMd5) {
       await LocalFileWriter.deleteFile(`${location}/${filterName(target)}`)
-      throw new WriteError({
-        message: 'md5 hash mismatch',
-        info: { sourceMd5, destMd5, location, target }
+      throw new WriteError('md5 hash mismatch', {
+        sourceMd5,
+        destMd5,
+        location,
+        target
       })
     }
   } finally {
@@ -100,9 +102,9 @@ async function getFileMd5Hash(location, filename) {
   try {
     return await _getFileMd5HashForPath(fullPath)
   } catch (err) {
-    throw new ReadError({
-      message: 'unable to get md5 hash from file',
-      info: { location, filename }
+    throw new ReadError('unable to get md5 hash from file', {
+      location,
+      filename
     }).withCause(err)
   }
 }

+ 5 - 3
services/filestore/app/js/FileController.js

@@ -75,9 +75,11 @@ function getFile(req, res, next) {
           res.end()
         } else if (err) {
           next(
-            new Errors.ReadError({
-              message: 'error transferring stream',
-              info: { bucket, key, format, style }
+            new Errors.ReadError('error transferring stream', {
+              bucket,
+              key,
+              format,
+              style
             }).withCause(err)
           )
         }

+ 7 - 6
services/filestore/app/js/FileConverter.js

@@ -69,9 +69,8 @@ async function preview(sourcePath) {
 
 async function _convert(sourcePath, requestedFormat, command) {
   if (!APPROVED_FORMATS.includes(requestedFormat)) {
-    throw new ConversionError({
-      message: 'invalid format requested',
-      info: { format: requestedFormat }
+    throw new ConversionError('invalid format requested', {
+      format: requestedFormat
     })
   }
 
@@ -87,9 +86,11 @@ async function _convert(sourcePath, requestedFormat, command) {
       timeout: FOURTY_SECONDS
     })
   } catch (err) {
-    throw new ConversionError({
-      message: 'something went wrong converting file',
-      info: { stderr: err.stderr, sourcePath, requestedFormat, destPath }
+    throw new ConversionError('something went wrong converting file', {
+      stderr: err.stderr,
+      sourcePath,
+      requestedFormat,
+      destPath
     }).withCause(err)
   }
 

+ 28 - 25
services/filestore/app/js/FileHandler.js

@@ -30,9 +30,10 @@ module.exports = {
 async function insertFile(bucket, key, stream) {
   const convertedKey = KeyBuilder.getConvertedFolderKey(key)
   if (!convertedKey.match(/^[0-9a-f]{24}\/([0-9a-f]{24}|v\/[0-9]+\/[a-z]+)/i)) {
-    throw new InvalidParametersError({
-      message: 'key does not match validation regex',
-      info: { bucket, key, convertedKey }
+    throw new InvalidParametersError('key does not match validation regex', {
+      bucket,
+      key,
+      convertedKey
     })
   }
   if (Settings.enableConversions) {
@@ -44,9 +45,10 @@ async function insertFile(bucket, key, stream) {
 async function deleteFile(bucket, key) {
   const convertedKey = KeyBuilder.getConvertedFolderKey(key)
   if (!convertedKey.match(/^[0-9a-f]{24}\/([0-9a-f]{24}|v\/[0-9]+\/[a-z]+)/i)) {
-    throw new InvalidParametersError({
-      message: 'key does not match validation regex',
-      info: { bucket, key, convertedKey }
+    throw new InvalidParametersError('key does not match validation regex', {
+      bucket,
+      key,
+      convertedKey
     })
   }
   const jobs = [PersistorManager.promises.deleteFile(bucket, key)]
@@ -58,9 +60,9 @@ async function deleteFile(bucket, key) {
 
 async function deleteProject(bucket, key) {
   if (!key.match(/^[0-9a-f]{24}\//i)) {
-    throw new InvalidParametersError({
-      message: 'key does not match validation regex',
-      info: { bucket, key }
+    throw new InvalidParametersError('key does not match validation regex', {
+      bucket,
+      key
     })
   }
   await PersistorManager.promises.deleteDirectory(bucket, key)
@@ -126,9 +128,11 @@ async function _getConvertedFileAndCache(bucket, key, convertedKey, opts) {
     )
   } catch (err) {
     LocalFileWriter.deleteFile(convertedFsPath, () => {})
-    throw new ConversionError({
-      message: 'failed to convert file',
-      info: { opts, bucket, key, convertedKey }
+    throw new ConversionError('failed to convert file', {
+      opts,
+      bucket,
+      key,
+      convertedKey
     }).withCause(err)
   }
   // Send back the converted file from the local copy to avoid problems
@@ -155,9 +159,10 @@ async function _convertFile(bucket, originalKey, opts) {
   try {
     originalFsPath = await _writeFileToDisk(bucket, originalKey, opts)
   } catch (err) {
-    throw new ConversionError({
-      message: 'unable to write file to disk',
-      info: { bucket, originalKey, opts }
+    throw new ConversionError('unable to write file to disk', {
+      bucket,
+      originalKey,
+      opts
     }).withCause(err)
   }
 
@@ -169,22 +174,20 @@ async function _convertFile(bucket, originalKey, opts) {
   } else if (opts.style === 'preview') {
     promise = FileConverter.promises.preview(originalFsPath)
   } else {
-    throw new ConversionError({
-      message: 'invalid file conversion options',
-      info: {
-        bucket,
-        originalKey,
-        opts
-      }
+    throw new ConversionError('invalid file conversion options', {
+      bucket,
+      originalKey,
+      opts
     })
   }
   let destPath
   try {
     destPath = await promise
   } catch (err) {
-    throw new ConversionError({
-      message: 'error converting file',
-      info: { bucket, originalKey, opts }
+    throw new ConversionError('error converting file', {
+      bucket,
+      originalKey,
+      opts
     }).withCause(err)
   }
   LocalFileWriter.deleteFile(originalFsPath, function() {})

+ 4 - 7
services/filestore/app/js/LocalFileWriter.js

@@ -30,9 +30,9 @@ async function writeStream(stream, key) {
   } catch (err) {
     await deleteFile(fsPath)
 
-    throw new WriteError({
-      message: 'problem writing file locally',
-      info: { err, fsPath }
+    throw new WriteError('problem writing file locally', {
+      err,
+      fsPath
     }).withCause(err)
   }
 }
@@ -45,10 +45,7 @@ async function deleteFile(fsPath) {
     await promisify(fs.unlink)(fsPath)
   } catch (err) {
     if (err.code !== 'ENOENT') {
-      throw new WriteError({
-        message: 'failed to delete file',
-        info: { fsPath }
-      }).withCause(err)
+      throw new WriteError('failed to delete file', { fsPath }).withCause(err)
     }
   }
 }

+ 8 - 8
services/filestore/app/js/MigrationPersistor.js

@@ -169,27 +169,27 @@ module.exports = function(primary, fallback) {
 
       await primary.promises.sendStream(destBucket, destKey, stream, sourceMd5)
     } catch (err) {
-      const error = new WriteError({
-        message: 'unable to copy file to destination persistor',
-        info: {
+      const error = new WriteError(
+        'unable to copy file to destination persistor',
+        {
           sourceBucket,
           destBucket,
           sourceKey,
           destKey
         }
-      }).withCause(err)
+      ).withCause(err)
       metrics.inc('fallback.copy.failure')
 
       try {
         await primary.promises.deleteFile(destBucket, destKey)
       } catch (err) {
-        error.info.cleanupError = new WriteError({
-          message: 'unable to clean up destination copy artifact',
-          info: {
+        error.info.cleanupError = new WriteError(
+          'unable to clean up destination copy artifact',
+          {
             destBucket,
             destKey
           }
-        }).withCause(err)
+        ).withCause(err)
       }
 
       logger.warn({ error }, 'failed to copy file from fallback')

+ 7 - 16
services/filestore/app/js/PersistorHelper.js

@@ -81,14 +81,11 @@ async function verifyMd5(persistor, bucket, key, sourceMd5, destMd5 = null) {
       logger.warn(err, 'error deleting file for invalid upload')
     }
 
-    throw new WriteError({
-      message: 'source and destination hashes do not match',
-      info: {
-        sourceMd5,
-        destMd5,
-        bucket,
-        key
-      }
+    throw new WriteError('source and destination hashes do not match', {
+      sourceMd5,
+      destMd5,
+      bucket,
+      key
     })
   }
 }
@@ -165,15 +162,9 @@ function wrapError(error, message, params, ErrorType) {
     ) ||
     (error.response && error.response.statusCode === 404)
   ) {
-    return new NotFoundError({
-      message: 'no such file',
-      info: params
-    }).withCause(error)
+    return new NotFoundError('no such file', params).withCause(error)
   } else {
-    return new ErrorType({
-      message: message,
-      info: params
-    }).withCause(error)
+    return new ErrorType(message, params).withCause(error)
   }
 }
 

+ 4 - 4
services/filestore/app/js/S3Persistor.js

@@ -322,10 +322,10 @@ function _getClientForBucket(bucket) {
     return _defaultClient
   }
 
-  throw new SettingsError({
-    message: 'no bucket-specific or default credentials provided',
-    info: { bucket }
-  })
+  throw new SettingsError(
+    'no bucket-specific or default credentials provided',
+    { bucket }
+  )
 }
 
 function _buildClientOptions(bucketCredentials) {

+ 4 - 7
services/filestore/app/js/SafeExec.js

@@ -42,13 +42,10 @@ function safeExec(command, options, callback) {
         process.kill(-child.pid, options.killSignal || 'SIGTERM')
       } catch (error) {
         cleanup(
-          new FailedCommandError({
-            message: 'failed to kill process after timeout',
-            info: {
-              command,
-              options,
-              pid: child.pid
-            }
+          new FailedCommandError('failed to kill process after timeout', {
+            command,
+            options,
+            pid: child.pid
           })
         )
       }

+ 7 - 3
services/filestore/package-lock.json

@@ -903,9 +903,8 @@
       }
     },
     "@overleaf/o-error": {
-      "version": "2.1.0",
-      "resolved": "https://registry.npmjs.org/@overleaf/o-error/-/o-error-2.1.0.tgz",
-      "integrity": "sha512-Zd9sks9LrLw8ErHt/cXeWIkyxWAqNAvNGn7wIjLQJH6TTEEW835PWOhpch+hQwwWsTxWIx/JDj+IpZ3ouw925g=="
+      "version": "git://github.com/overleaf/o-error.git#14e515a195d6dbd3711c5a211a730752802d3b03",
+      "from": "git://github.com/overleaf/o-error.git#14e515a195d6dbd3711c5a211a730752802d3b03"
     },
     "@protobufjs/aspromise": {
       "version": "1.1.2",
@@ -3480,6 +3479,11 @@
         "yn": "^3.1.1"
       },
       "dependencies": {
+        "@overleaf/o-error": {
+          "version": "2.1.0",
+          "resolved": "https://registry.npmjs.org/@overleaf/o-error/-/o-error-2.1.0.tgz",
+          "integrity": "sha512-Zd9sks9LrLw8ErHt/cXeWIkyxWAqNAvNGn7wIjLQJH6TTEEW835PWOhpch+hQwwWsTxWIx/JDj+IpZ3ouw925g=="
+        },
         "qs": {
           "version": "6.5.2",
           "resolved": "https://registry.npmjs.org/qs/-/qs-6.5.2.tgz",

+ 1 - 1
services/filestore/package.json

@@ -21,7 +21,7 @@
   },
   "dependencies": {
     "@google-cloud/storage": "^4.7.0",
-    "@overleaf/o-error": "^2.1.0",
+    "@overleaf/o-error": "git://github.com/overleaf/o-error.git#14e515a195d6dbd3711c5a211a730752802d3b03",
     "aws-sdk": "^2.648.0",
     "body-parser": "^1.19.0",
     "express": "^4.17.1",

+ 1 - 1
services/filestore/test/unit/js/GcsPersistorTests.js

@@ -44,6 +44,7 @@ describe('GcsPersistorTests', function() {
           user_files: 'user_files'
         },
         gcs: {
+          deleteConcurrency: 1,
           directoryKeyRegex: /^[0-9a-fA-F]{24}\/[0-9a-fA-F]{24}/
         }
       }
@@ -526,7 +527,6 @@ describe('GcsPersistorTests', function() {
     const directoryName = `${ObjectId()}/${ObjectId()}`
     describe('with valid parameters', function() {
       beforeEach(async function() {
-        console.log(key)
         return GcsPersistor.promises.deleteDirectory(bucket, directoryName)
       })