From 81553132e35c319288b49d93ec123a74cd698368 Mon Sep 17 00:00:00 2001 From: Joan Gallego Girona Date: Fri, 20 Sep 2024 10:50:37 +0200 Subject: [PATCH 1/2] Log s3 timeout as debug (#7247) * Modify workflows to not run on staging and production only release runs for the belonging branch * Change number of maximum training samples for metadata extractor * catch s3 timeoutError catch and rethrow a custom S3TimeoutError S3TimeoutError is logged as debug --------- Co-authored-by: Gabo --- app/api/files/S3Storage.ts | 83 +++++++++++++++---------- app/api/files/specs/s3Storage.spec.ts | 45 ++++++++++++++ app/api/files/storage.ts | 18 +++++- app/api/utils/handleError.js | 5 ++ app/api/utils/specs/handleError.spec.js | 12 ++++ 5 files changed, 127 insertions(+), 36 deletions(-) create mode 100644 app/api/files/specs/s3Storage.spec.ts diff --git a/app/api/files/S3Storage.ts b/app/api/files/S3Storage.ts index 6038e9755e..9d5023fcfe 100644 --- a/app/api/files/S3Storage.ts +++ b/app/api/files/S3Storage.ts @@ -6,23 +6,33 @@ import { S3Client, _Object, } from '@aws-sdk/client-s3'; -import { NodeHttpHandler } from '@smithy/node-http-handler'; import { config } from 'api/config'; +class S3TimeoutError extends Error { + readonly s3TimeoutError: Error; + + constructor(s3TimeoutError: Error) { + super(s3TimeoutError.message); + this.s3TimeoutError = s3TimeoutError; + } +} + +const catchS3Errors = async (cb: () => Promise): Promise => { + try { + return await cb(); + } catch (err) { + if (err.name === 'TimeoutError') { + throw new S3TimeoutError(err); + } + throw err; + } +}; + export class S3Storage { private client: S3Client; - constructor() { - this.client = new S3Client({ - requestHandler: new NodeHttpHandler({ - socketTimeout: 30000, - }), - apiVersion: 'latest', - region: 'placeholder-region', - endpoint: config.s3.endpoint, - credentials: config.s3.credentials, - forcePathStyle: true, - }); + constructor(s3Client: S3Client) { + this.client = s3Client; } static bucketName() { @@ -30,20 +40,22 @@ export class S3Storage { } async upload(key: string, body: Buffer) { - return this.client.send( - new PutObjectCommand({ Bucket: S3Storage.bucketName(), Key: key, Body: body }) + return catchS3Errors(async () => + this.client.send( + new PutObjectCommand({ Bucket: S3Storage.bucketName(), Key: key, Body: body }) + ) ); } async get(key: string) { - const response = await this.client.send( - new GetObjectCommand({ - Bucket: S3Storage.bucketName(), - Key: key, - }) + return catchS3Errors(async () => + this.client.send( + new GetObjectCommand({ + Bucket: S3Storage.bucketName(), + Key: key, + }) + ) ); - - return response; } async list(prefix?: string) { @@ -61,21 +73,26 @@ export class S3Storage { return response.NextContinuationToken; }; - let continuationToken = await requestNext(); - while (continuationToken) { - // eslint-disable-next-line no-await-in-loop - continuationToken = await requestNext(continuationToken); - } - - return objects; + return catchS3Errors(async () => { + let continuationToken = await requestNext(); + while (continuationToken) { + // eslint-disable-next-line no-await-in-loop + continuationToken = await requestNext(continuationToken); + } + return objects; + }); } async delete(key: string) { - await this.client.send( - new DeleteObjectCommand({ - Bucket: S3Storage.bucketName(), - Key: key, - }) + return catchS3Errors(async () => + this.client.send( + new DeleteObjectCommand({ + Bucket: S3Storage.bucketName(), + Key: key, + }) + ) ); } } + +export { S3TimeoutError }; diff --git a/app/api/files/specs/s3Storage.spec.ts b/app/api/files/specs/s3Storage.spec.ts new file mode 100644 index 0000000000..35c601fe97 --- /dev/null +++ b/app/api/files/specs/s3Storage.spec.ts @@ -0,0 +1,45 @@ +import { S3Storage, S3TimeoutError } from '../S3Storage'; + +let s3Storage: S3Storage; + +class S3TimeoutClient { + // eslint-disable-next-line class-methods-use-this + send() { + const error = new Error(); + error.name = 'TimeoutError'; + throw error; + } +} + +describe('s3Storage', () => { + beforeAll(async () => { + // @ts-ignore + s3Storage = new S3Storage(new S3TimeoutClient()); + }); + + describe('get', () => { + it('should throw S3TimeoutError on timeout', async () => { + await expect(s3Storage.get('dummy_key')).rejects.toBeInstanceOf(S3TimeoutError); + }); + }); + + describe('upload', () => { + it('should throw S3TimeoutError on timeout', async () => { + await expect( + s3Storage.upload('dummy_key', Buffer.from('dummy buffer', 'utf-8')) + ).rejects.toBeInstanceOf(S3TimeoutError); + }); + }); + + describe('delete', () => { + it('should throw S3TimeoutError on timeout', async () => { + await expect(s3Storage.delete('dummy_key')).rejects.toBeInstanceOf(S3TimeoutError); + }); + }); + + describe('list', () => { + it('should throw S3TimeoutError on timeout', async () => { + await expect(s3Storage.list()).rejects.toBeInstanceOf(S3TimeoutError); + }); + }); +}); diff --git a/app/api/files/storage.ts b/app/api/files/storage.ts index 2dfac22938..ba43d25b75 100644 --- a/app/api/files/storage.ts +++ b/app/api/files/storage.ts @@ -1,6 +1,7 @@ -import { NoSuchKey } from '@aws-sdk/client-s3'; +import { NoSuchKey, S3Client } from '@aws-sdk/client-s3'; import { config } from 'api/config'; import { tenants } from 'api/tenants'; +import { NodeHttpHandler } from '@smithy/node-http-handler'; // eslint-disable-next-line node/no-restricted-import import { createReadStream, createWriteStream } from 'fs'; // eslint-disable-next-line node/no-restricted-import @@ -9,6 +10,7 @@ import path from 'path'; import { FileType } from 'shared/types/fileType'; import { Readable } from 'stream'; import { pipeline } from 'stream/promises'; +import { FileNotFound } from './FileNotFound'; import { activityLogPath, attachmentsPath, @@ -18,14 +20,24 @@ import { uploadsPath, } from './filesystem'; import { S3Storage } from './S3Storage'; -import { FileNotFound } from './FileNotFound'; type FileTypes = NonNullable | 'activitylog' | 'segmentation'; let s3Instance: S3Storage; const s3 = () => { if (config.s3.endpoint && !s3Instance) { - s3Instance = new S3Storage(); + s3Instance = new S3Storage( + new S3Client({ + requestHandler: new NodeHttpHandler({ + socketTimeout: 30000, + }), + apiVersion: 'latest', + region: 'placeholder-region', + endpoint: config.s3.endpoint, + credentials: config.s3.credentials, + forcePathStyle: true, + }) + ); } return s3Instance; }; diff --git a/app/api/utils/handleError.js b/app/api/utils/handleError.js index 38a8259773..d2690f2d65 100644 --- a/app/api/utils/handleError.js +++ b/app/api/utils/handleError.js @@ -5,6 +5,7 @@ import { appContext } from 'api/utils/AppContext'; import { UnauthorizedError } from 'api/authorization.v2/errors/UnauthorizedError'; import { ValidationError } from 'api/common.v2/validation/ValidationError'; import { FileNotFound } from 'api/files/FileNotFound'; +import { S3TimeoutError } from 'api/files/S3Storage'; const ajvPrettifier = error => { const errorMessage = [error.message]; @@ -62,6 +63,10 @@ const prettifyError = (error, { req = {}, uncaught = false } = {}) => { result = { code: 500, message: error.stack, logLevel: 'error' }; } + if (error instanceof S3TimeoutError) { + result = { code: 408, message: `${error.message}\n${error.stack}`, logLevel: 'debug' }; + } + if (error instanceof Ajv.ValidationError) { result = { code: 422, message: error.message, validations: error.errors, logLevel: 'debug' }; } diff --git a/app/api/utils/specs/handleError.spec.js b/app/api/utils/specs/handleError.spec.js index bd7774956d..da496cf627 100644 --- a/app/api/utils/specs/handleError.spec.js +++ b/app/api/utils/specs/handleError.spec.js @@ -4,6 +4,7 @@ import { legacyLogger } from 'api/log'; import { errors as elasticErrors } from '@elastic/elasticsearch'; import { appContext } from 'api/utils/AppContext'; import { handleError, prettifyError } from '../handleError'; +import { S3TimeoutError } from 'api/files/S3Storage'; const contextRequestId = '1234'; @@ -18,6 +19,17 @@ describe('handleError', () => { }); describe('errors by type', () => { + describe('and is instance of S3TimeoutError', () => { + it('should be a debug logLevel and a 408 http code', () => { + const errorInstance = new S3TimeoutError(new Error('timeout')); + const error = handleError(errorInstance); + expect(error).toMatchObject({ + code: 408, + logLevel: 'debug', + }); + expect(legacyLogger.debug.mock.calls[0][0]).toContain('timeout'); + }); + }); describe('when error is instance of Error', () => { it('should return the error with 500 code without the original error and error stack', () => { const errorInstance = new Error('error'); From 127c449167f3204798ddd49f24b485282decabba Mon Sep 17 00:00:00 2001 From: Daneryl Date: Fri, 20 Sep 2024 10:50:42 +0200 Subject: [PATCH 2/2] Bump version --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 94cb07a52f..58c835c38c 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "uwazi", - "version": "1.184.4", + "version": "1.184.5", "description": "Uwazi is a free, open-source solution for organising, analysing and publishing your documents.", "keywords": [ "react"