From 4094ab58aaca558f120186633fa138b9f9b9b684 Mon Sep 17 00:00:00 2001
From: Kagami Sascha Rosylight <saschanaz@outlook.com>
Date: Sat, 11 Mar 2023 05:35:51 +0100
Subject: [PATCH] fix(backend/DriveService): gracely skip when getting
 NoSuchKey error from S3 (#10292)

* fix(backend/DriveService): gracely skip when getting NoSuchKey error from S3

* Update CHANGELOG.md

* import type
---
 CHANGELOG.md                               |  1 +
 packages/backend/src/core/DriveService.ts  | 22 ++++++---
 packages/backend/test/unit/DriveService.ts | 55 ++++++++++++++++++++++
 3 files changed, 72 insertions(+), 6 deletions(-)
 create mode 100644 packages/backend/test/unit/DriveService.ts

diff --git a/CHANGELOG.md b/CHANGELOG.md
index f6e64bd09f..466608dcab 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -28,6 +28,7 @@ You should also include the user name that made the change.
 - ロールで広告を無効にするとadmin/adsでプレビューがでてこない問題を修正
 - /api-consoleページにアクセスすると404が出る問題を修正
 - SMTP Login id length is too short
+- AWS S3からのファイル削除でNoSuchKeyエラーが出ると進めらない状態になる問題を修正
 
 ## 13.9.2 (2023/03/06)
 
diff --git a/packages/backend/src/core/DriveService.ts b/packages/backend/src/core/DriveService.ts
index c3835faa33..dfaacffc1d 100644
--- a/packages/backend/src/core/DriveService.ts
+++ b/packages/backend/src/core/DriveService.ts
@@ -33,8 +33,8 @@ import { UserEntityService } from '@/core/entities/UserEntityService.js';
 import { FileInfoService } from '@/core/FileInfoService.js';
 import { bindThis } from '@/decorators.js';
 import { RoleService } from '@/core/RoleService.js';
-import type S3 from 'aws-sdk/clients/s3.js';
 import { correctFilename } from '@/misc/correct-filename.js';
+import type S3 from 'aws-sdk/clients/s3.js';
 
 type AddFileArgs = {
 	/** User who wish to add file */
@@ -476,7 +476,7 @@ export class DriveService {
 			// DriveFile.nameは256文字, validateFileNameは200文字制限であるため、
 			// extを付加してデータベースの文字数制限に当たることはまずない
 			(name && this.driveFileEntityService.validateFileName(name)) ? name : 'untitled',
-			info.type.ext
+			info.type.ext,
 		);
 
 		if (user && !force) {
@@ -728,10 +728,20 @@ export class DriveService {
 
 		const s3 = this.s3Service.getS3(meta);
 
-		await s3.deleteObject({
-			Bucket: meta.objectStorageBucket!,
-			Key: key,
-		}).promise();
+		try {
+			await s3.deleteObject({
+				Bucket: meta.objectStorageBucket!,
+				Key: key,
+			}).promise();
+		} catch (err: any) {
+			if (err.code === 'NoSuchKey') {
+				console.warn(`The object storage had no such key to delete: ${key}. Skipping this.`, err);
+				return;
+			}
+			throw new Error(`Failed to delete the file from the object storage with the given key: ${key}`, {
+				cause: err,
+			});
+		}
 	}
 
 	@bindThis
diff --git a/packages/backend/test/unit/DriveService.ts b/packages/backend/test/unit/DriveService.ts
new file mode 100644
index 0000000000..0549800a68
--- /dev/null
+++ b/packages/backend/test/unit/DriveService.ts
@@ -0,0 +1,55 @@
+process.env.NODE_ENV = 'test';
+
+import { jest } from '@jest/globals';
+import { Test } from '@nestjs/testing';
+import { GlobalModule } from '@/GlobalModule.js';
+import { DriveService } from '@/core/DriveService.js';
+import { CoreModule } from '@/core/CoreModule.js';
+import { S3Service } from '@/core/S3Service';
+import type { Meta } from '@/models';
+import type { DeleteObjectOutput } from 'aws-sdk/clients/s3';
+import type { AWSError } from 'aws-sdk/lib/error';
+import type { PromiseResult, Request } from 'aws-sdk/lib/request';
+import type { TestingModule } from '@nestjs/testing';
+
+describe('DriveService', () => {
+	let app: TestingModule;
+	let driveService: DriveService;
+
+	beforeEach(async () => {
+		app = await Test.createTestingModule({
+			imports: [GlobalModule, CoreModule],
+			providers: [DriveService, S3Service],
+		}).compile();
+		app.enableShutdownHooks();
+		driveService = app.get<DriveService>(DriveService);
+
+		const s3Service = app.get<S3Service>(S3Service);
+		const s3 = s3Service.getS3({} as Meta);
+
+		// new S3() surprisingly does not return an instance of class S3.
+		// Let's use getPrototypeOf here to get a real prototype, since spying on S3.prototype doesn't work.
+		// TODO: Use `aws-sdk-client-mock` package when upgrading to AWS SDK v3.
+		jest.spyOn(Object.getPrototypeOf(s3), 'deleteObject').mockImplementation(() => {
+			// Roughly mock AWS request object
+			return {
+				async promise(): Promise<PromiseResult<DeleteObjectOutput, AWSError>> {
+					const err = new Error('mock') as AWSError;
+					err.code = 'NoSuchKey';
+					throw err;
+				},
+			} as Request<DeleteObjectOutput, AWSError>;
+		});
+	});
+
+	describe('Object storage', () => {
+		test('delete a file with no valid key', async () => {
+			try {
+				await driveService.deleteObjectStorageFile('lol no way');
+			} catch (err: any) {
+				console.log(err.cause);
+				throw err;
+			}
+		});
+	});
+});