Ensure sync operation does not fail when unable to write tags to s3

There are edge case scenarios where the PutObjectTaggingCommand will fail due to some upstream S3 configuration. We want to ensure that in the event sync is unable to write a coms-id tag to an object, the overall sync operation will still carry on instead of halting and failing out. Signed-off-by: Jeremy Ho <[email protected]>
bcgov · Dec 13, 2023 · 4b30cd3 · 4b30cd3
1 parent 4904784
commit 4b30cd3
Show file tree

Hide file tree

Showing 2 changed files with 80 additions and 11 deletions.
diff --git a/app/src/services/sync.js b/app/src/services/sync.js
@@ -42,6 +42,8 @@ const service = {
             filePath: path,
             bucketId: bucketId,
             tags: TagSet.concat([{ Key: 'coms-id', Value: objId }])
+          }).catch((err) => {
+            log.warn(`Unable to add coms-id tag: ${err.message}`, { function: '_deriveObjectId' });
           });
         }
       }
@@ -354,17 +356,21 @@ const service = {
          * NOTE: adding tags to a specified version (passing a `VersionId` parameter) will affect `Last Modified`
          * attribute of multiple versions on some s3 storage providors including Dell ECS
          */
-        await storageService.putObjectTagging({
-          filePath: path,
-          tags: (s3TagsForVersion?.TagSet ?? []).concat([{
-            Key: 'coms-id',
-            Value: comsVersion.objectId
-          }]),
-          bucketId: bucketId,
-          // s3VersionId: comsVersion.s3VersionId,
-        });
-        // add to our arrays for comaprison
-        s3Tags.push({ key: 'coms-id', value: comsVersion.objectId });
+        try {
+          await storageService.putObjectTagging({
+            filePath: path,
+            tags: (s3TagsForVersion?.TagSet ?? []).concat([{
+              Key: 'coms-id',
+              Value: comsVersion.objectId
+            }]),
+            bucketId: bucketId,
+            // s3VersionId: comsVersion.s3VersionId,
+          });
+          // add to our arrays for comaprison
+          s3Tags.push({ key: 'coms-id', value: comsVersion.objectId });
+        } catch (err) {
+          log.warn(`Unable to add coms-id tag: ${err.message}`, { function: 'syncTags' });
+        }
       }
 
       // Dissociate Tags not in S3

diff --git a/app/tests/unit/services/sync.spec.js b/app/tests/unit/services/sync.spec.js
@@ -86,6 +86,7 @@ describe('_deriveObjectId', () => {
       getObjectTaggingSpy.mockResolvedValue({
         TagSet: [{ Key: 'coms-id', Value: 'garbage' }]
       });
+      putObjectTaggingSpy.mockResolvedValue(true);
 
       const result = await service._deriveObjectId({}, path, bucketId);
 
@@ -106,8 +107,33 @@ describe('_deriveObjectId', () => {
       }));
     });
 
+
     it('Returns a new uuid if unavailable and pushes tags when less than 10 present', async () => {
       getObjectTaggingSpy.mockResolvedValue({ TagSet: [] });
+      putObjectTaggingSpy.mockResolvedValue(true);
+
+      const result = await service._deriveObjectId({}, path, bucketId);
+
+      expect(result).toBeTruthy();
+      expect(typeof result).toBe('string');
+      expect(result).toMatch(uuidv4Regex);
+      expect(getObjectTaggingSpy).toHaveBeenCalledTimes(1);
+      expect(getObjectTaggingSpy).toHaveBeenCalledWith(expect.objectContaining({
+        filePath: path,
+        bucketId: bucketId
+      }));
+      expect(listAllObjectVersionsSpy).toHaveBeenCalledTimes(0);
+      expect(putObjectTaggingSpy).toHaveBeenCalledTimes(1);
+      expect(putObjectTaggingSpy).toHaveBeenCalledWith(expect.objectContaining({
+        filePath: path,
+        bucketId: bucketId,
+        tags: expect.any(Array)
+      }));
+    });
+
+    it('Returns a new uuid if unavailable and continues when unable to write to S3', async () => {
+      getObjectTaggingSpy.mockResolvedValue({ TagSet: [] });
+      putObjectTaggingSpy.mockRejectedValue({ message: 'nope' });
 
       const result = await service._deriveObjectId({}, path, bucketId);
 
@@ -1045,6 +1071,43 @@ describe('syncTags', () => {
     expect(versionTrx.commit).toHaveBeenCalledTimes(1);
   });
 
+  it('should not write coms-id tag when coms version is latest and continues when unable to write to S3', async () => {
+    fetchTagsForVersionSpy.mockResolvedValue([{}]);
+    getObjectTaggingSpy.mockResolvedValue({});
+    putObjectTaggingSpy.mockRejectedValue({ message: 'nope' });
+
+    const result = await service.syncTags(comsVersion, path, bucketId);
+
+    expect(result).toBeTruthy();
+    expect(Array.isArray(result)).toBeTruthy();
+    expect(result).toHaveLength(0);
+
+    expect(Version.startTransaction).toHaveBeenCalledTimes(1);
+    expect(associateTagsSpy).toHaveBeenCalledTimes(0);
+    expect(dissociateTagsSpy).toHaveBeenCalledTimes(0);
+    expect(fetchTagsForVersionSpy).toHaveBeenCalledTimes(1);
+    expect(fetchTagsForVersionSpy).toHaveBeenCalledWith(expect.objectContaining({
+      versionIds: comsVersion.id
+    }), expect.any(Object));
+    expect(getObjectTaggingSpy).toHaveBeenCalledTimes(1);
+    expect(getObjectTaggingSpy).toHaveBeenCalledWith(expect.objectContaining({
+      filePath: path,
+      s3VersionId: comsVersion.s3VersionId,
+      bucketId: bucketId
+    }));
+    expect(getSpy).toHaveBeenCalledTimes(0);
+    expect(putObjectTaggingSpy).toHaveBeenCalledTimes(1);
+    expect(putObjectTaggingSpy).toHaveBeenCalledWith(expect.objectContaining({
+      filePath: path,
+      tags: expect.arrayContaining([{
+        Key: 'coms-id',
+        Value: comsVersion.objectId
+      }]),
+      bucketId: bucketId,
+    }));
+    expect(versionTrx.commit).toHaveBeenCalledTimes(1);
+  });
+
   it('should not write coms-id tag when there are already 10 tags', async () => {
     fetchTagsForVersionSpy.mockResolvedValue([{}]);
     getObjectTaggingSpy.mockResolvedValue({