Merge pull request #3110 from processing/fix/s3-errors

Fix/s3 errors

Author: raclim

server/controllers/aws.controller.js

@@ -52,13 +52,15 @@ export async function deleteObjectsFromS3(keyList) {
     try {
       await s3Client.send(new DeleteObjectsCommand(params));
     } catch (error) {
-      if (error.name === 'NotFound') {
-        console.log('Object does not exist:', error);
-      } else {
-        console.error('Error deleting objects from S3: ', error);
+      if (error instanceof TypeError) {
+        return null;
       }
+      console.error('Error deleting objects from S3: ', error);
+      throw error;
     }
   }
+
+  return objectsToDelete;
 }
 
 export async function deleteObjectFromS3(req, res) {
@@ -110,12 +112,12 @@ export async function copyObjectInS3(url, userId) {
   try {
     await s3Client.send(new HeadObjectCommand(headParams));
   } catch (error) {
-    if (error.name === 'NotFound') {
-      console.log('Object does not exist:', error);
-    } else {
-      console.error('Error fetching object metadata:', error);
-      throw error;
+    // temporary error handling for sketches with missing assets
+    if (error instanceof TypeError) {
+      return null;
     }
+    console.error('Error retrieving object metadat:', error);
+    throw error;
   }
 
   const params = {
@@ -127,16 +129,16 @@ export async function copyObjectInS3(url, userId) {
 
   try {
     await s3Client.send(new CopyObjectCommand(params));
-    return `${s3Bucket}${userId}/${newFilename}`;
   } catch (error) {
     // temporary error handling for sketches with missing assets
-    if (error.startsWith('TypeError')) {
-      console.log('Object does not exist:', error);
+    if (error instanceof TypeError) {
       return null;
     }
     console.error('Error copying object:', error);
     throw error;
   }
+
+  return `${s3Bucket}${userId}/${newFilename}`;
 }
 
 export async function copyObjectInS3RequestHandler(req, res) {
@@ -227,8 +229,7 @@ export async function listObjectsInS3ForUser(userId) {
 
     return { assets: projectAssets, totalSize };
   } catch (error) {
-    if (error.name === 'NotFound') {
-      console.log('Object does not exist:', error);
+    if (error instanceof TypeError) {
       return null;
     }
     console.error('Got an error: ', error);

server/models/user.js

@@ -177,18 +177,27 @@ userSchema.methods.comparePassword = async function comparePassword(
 /**
  * Helper method for validating a user's api key
  */
-userSchema.methods.findMatchingKey = function findMatchingKey(
-  candidateKey,
-  cb
+userSchema.methods.findMatchingKey = async function findMatchingKey(
+  candidateKey
 ) {
-  let foundOne = false;
-  this.apiKeys.forEach((k) => {
-    if (bcrypt.compareSync(candidateKey, k.hashedKey)) {
-      foundOne = true;
-      cb(null, true, k);
+  let keyObj = { isMatch: false, keyDocument: null };
+  /* eslint-disable no-restricted-syntax */
+  for (const k of this.apiKeys) {
+    try {
+      /* eslint-disable no-await-in-loop */
+      const foundOne = await bcrypt.compareSync(candidateKey, k.hashedKey);
+
+      if (foundOne) {
+        keyObj = { isMatch: true, keyDocument: k };
+        return keyObj;
+      }
+    } catch (error) {
+      console.error('Matching API key not found !');
+      return keyObj;
     }
-  });
-  if (!foundOne) cb('Matching API key not found !', false, null);
+  }
+
+  return keyObj;
 };
 
 /**
@@ -197,7 +206,7 @@ userSchema.methods.findMatchingKey = function findMatchingKey(
  *
  * @param {string|string[]} email - Email string or array of email strings
  * @callback [cb] - Optional error-first callback that passes User document
- * @return {Promise<Object>} - Returns Promise fulfilled by User document
+ * @return {Object} - Returns User Object fulfilled by User document
  */
 userSchema.statics.findByEmail = async function findByEmail(email) {
   const user = this;
@@ -240,7 +249,7 @@ userSchema.statics.findAllByEmails = async function findAllByEmails(emails) {
  * @param {string} username - Username string
  * @param {Object} [options] - Optional options
  * @param {boolean} options.caseInsensitive - Does a caseInsensitive query, defaults to false
- * @return {Promise<Object>} - Returns Promise fulfilled by User document
+ * @return {Object} - Returns User Object fulfilled by User document
  */
 userSchema.statics.findByUsername = async function findByUsername(
   username,
@@ -279,7 +288,7 @@ userSchema.statics.findByUsername = async function findByUsername(
  *                                          default query for username or email, defaults
  *                                          to false
  * @param {("email"|"username")} options.valueType - Prevents automatic type inferrence
- * @return {Promise<Object>} - Returns Promise fulfilled by User document
+ * @return {Object} - Returns User Object fulfilled by User document
  */
 userSchema.statics.findByEmailOrUsername = async function findByEmailOrUsername(
   value,
@@ -321,18 +330,22 @@ userSchema.statics.findByEmailOrUsername = async function findByEmailOrUsername(
  *
  * @param {string} email
  * @param {string} username
- * @callback [cb] - Optional error-first callback that passes User document
- * @return {Promise<Object>} - Returns Promise fulfilled by User document
+ * @return {Object} - Returns User Object fulfilled by User document
  */
-userSchema.statics.findByEmailAndUsername = function findByEmailAndUsername(
+userSchema.statics.findByEmailAndUsername = async function findByEmailAndUsername(
   email,
-  username,
-  cb
+  username
 ) {
+  const user = this;
   const query = {
     $or: [{ email }, { username }]
   };
-  return this.findOne(query).collation({ locale: 'en', strength: 2 }).exec(cb);
+  const foundUser = await user
+    .findOne(query)
+    .collation({ locale: 'en', strength: 2 })
+    .exec();
+
+  return foundUser;
 };
 
 userSchema.statics.EmailConfirmation = EmailConfirmationStates;