Fix error logging (scalabel#211)

* fix s3 config * return err, not str, and add test * fix s3 config * handle s3 storage creation errors * add logging * fix test * use throw in async function * fix build Co-authored-by: Fisher Yu <[email protected]>
Jinming-Su · Aug 15, 2020 · b191681 · b191681
1 parent da275ea
commit b191681
Show file tree

Hide file tree

Showing 4 changed files with 21 additions and 9 deletions.
diff --git a/app/config/s3_config.yml b/app/config/s3_config.yml
@@ -3,5 +3,6 @@ http:
     port: 8686
 storage:
     data: "us-west-2:scalabel-unit-testing/data"
-    database: "s3"
+    type: "s3"
+    itemDir: "./"
 ...
diff --git a/app/src/server/listeners.ts b/app/src/server/listeners.ts
@@ -156,16 +156,21 @@ export class Listeners {
     if (req.body === undefined ||
         req.body.fields === undefined ||
         req.body.files === undefined) {
-      this.badFormResponse(res)
-      return
+      return this.badFormResponse(res)
     }
 
     /**
      * Use the region/bucket specified in the request
      * to access the item/category/attribute files
      */
     const s3Path = req.body.fields.s3_path as string
-    const storage = new S3Storage(s3Path)
+    let storage: Storage
+    try {
+      storage = new S3Storage(s3Path)
+    } catch (err) {
+      Logger.error(err)
+      return this.badFormResponse(res)
+    }
     storage.setExt('')
     await this.createProjectFromDicts(
       storage, req.body.fields, req.body.files, false, res)
@@ -181,8 +186,7 @@ export class Listeners {
     }
 
     if (req.fields === undefined || req.files === undefined) {
-      this.badFormResponse(res)
-      return
+      return this.badFormResponse(res)
     }
 
     const fields = req.fields as { [key: string]: string}

diff --git a/app/src/server/s3_storage.ts b/app/src/server/s3_storage.ts
@@ -220,17 +220,17 @@ export class S3Storage extends Storage {
    * @param {string} key: relative path of file
    */
   public async load (key: string): Promise<string> {
-    const params = {
+    const params: AWS.S3.GetObjectRequest = {
       Bucket: this.bucketName,
       Key: this.fullFile(key)
     }
 
     if (!await this.hasKey(key)) {
-      return Promise.reject('Key does not exist')
+      throw new Error(`Key '${params.Key}' does not exist`)
     }
     const data = await this.s3.getObject(params).promise()
     if (!data || !data.Body) {
-      return Promise.reject(Error('No data at key'))
+      throw new Error(`No data at key '${params.Key}'`)
     }
 
     return data.Body.toString()

diff --git a/app/test/server/s3_storage.test.ts b/app/test/server/s3_storage.test.ts
@@ -102,6 +102,13 @@ describe('test s3 storage', () => {
     })
   })
 
+  test('loading nonexistent key', async () => {
+    await storage.load('not_a_real_key').catch((e: Error) => {
+      expect(e.message).toEqual(
+        `Key '${storageName}/not_a_real_key.json' does not exist`)
+    })
+  })
+
   test('save then load', () => {
     const taskId = index2str(2)
     const key = getTaskKey(projectName, taskId)