From f8dbab314601590f8e87c52fc64599d114f96829 Mon Sep 17 00:00:00 2001 From: filou Date: Tue, 18 Mar 2025 16:01:18 +0100 Subject: [PATCH 1/3] [Conversations - Fragment] Return 400 on image format not supported Description --- Fixes issue from thread [here](), so there is no unhandled api error. Receiving a 3rd party message "Image format not supported", an unhandled api error is not warranted since not actionable by us. The best is to throw a 400. Risks --- low Deploy --- front --- .../conversation/content_fragment.ts | 12 +++++-- front/lib/api/files/upload.ts | 31 ++++++++++--------- .../conversations/[cId]/content_fragments.ts | 12 +++++-- .../w/[wId]/assistant/conversations/index.ts | 9 ++++++ 4 files changed, 45 insertions(+), 19 deletions(-) diff --git a/front/lib/api/assistant/conversation/content_fragment.ts b/front/lib/api/assistant/conversation/content_fragment.ts index 9d87bed5f8f9..398974dc0bf0 100644 --- a/front/lib/api/assistant/conversation/content_fragment.ts +++ b/front/lib/api/assistant/conversation/content_fragment.ts @@ -1,4 +1,8 @@ -import { processAndStoreFile } from "@app/lib/api/files/upload"; +import type { + ProcessAndStoreFileError} from "@app/lib/api/files/upload"; +import { + processAndStoreFile +} from "@app/lib/api/files/upload"; import type { Authenticator } from "@app/lib/auth"; import { FileResource } from "@app/lib/resources/file_resource"; import { getResourceIdFromSId } from "@app/lib/resources/string_ids"; @@ -38,7 +42,9 @@ export async function toFileContentFragment( contentFragment: ContentFragmentInputWithContentType; fileName?: string; } -): Promise> { +): Promise< + Result +> { const file = await FileResource.makeNew({ contentType: contentFragment.contentType, fileName: @@ -58,8 +64,10 @@ export async function toFileContentFragment( if (processRes.isErr()) { return new Err({ + name: "dust_error", message: `Error creating file for content fragment: ` + processRes.error.message, + code: processRes.error.code, }); } diff --git a/front/lib/api/files/upload.ts b/front/lib/api/files/upload.ts index f246bbf53cf5..20f611d94c86 100644 --- a/front/lib/api/files/upload.ts +++ b/front/lib/api/files/upload.ts @@ -91,7 +91,6 @@ const resizeAndUploadToFileStorage: ProcessingFunction = async ( try { await pipeline(readStream, resizedImageStream, writeStream); - return new Ok(undefined); } catch (err) { logger.error( @@ -333,25 +332,21 @@ const maybeApplyProcessing: ProcessingFunction = async ( } }; +export type ProcessAndStoreFileError = Omit & { + code: + | "internal_server_error" + | "invalid_request_error" + | "file_too_large" + | "file_type_not_supported" + | "file_is_empty"; +}; export async function processAndStoreFile( auth: Authenticator, { file, reqOrString, }: { file: FileResource; reqOrString: IncomingMessage | string } -): Promise< - Result< - FileResource, - Omit & { - code: - | "internal_server_error" - | "invalid_request_error" - | "file_too_large" - | "file_type_not_supported" - | "file_is_empty"; - } - > -> { +): Promise> { if (file.isReady || file.isFailed) { return new Err({ name: "dust_error", @@ -389,10 +384,16 @@ export async function processAndStoreFile( const processingRes = await maybeApplyProcessing(auth, file); if (processingRes.isErr()) { await file.markAsFailed(); + // Unfortunately, there is no better way to catch this image format error. + const code = processingRes.error.message.includes( + "Input buffer contains unsupported image format" + ) + ? "file_type_not_supported" + : "internal_server_error"; return new Err({ name: "dust_error", - code: "invalid_request_error", + code, message: `Failed to process the file : ${processingRes.error}`, }); } diff --git a/front/pages/api/v1/w/[wId]/assistant/conversations/[cId]/content_fragments.ts b/front/pages/api/v1/w/[wId]/assistant/conversations/[cId]/content_fragments.ts index 477ebbe562fa..8543d650ca99 100644 --- a/front/pages/api/v1/w/[wId]/assistant/conversations/[cId]/content_fragments.ts +++ b/front/pages/api/v1/w/[wId]/assistant/conversations/[cId]/content_fragments.ts @@ -114,7 +114,7 @@ async function handler( } } const { context, ...rest } = r.data; - let contentFragment = rest; + const contentFragment = rest; // If we receive a content fragment that is not file based, we transform it to a file-based // one. @@ -123,9 +123,17 @@ async function handler( contentFragment, }); if (contentFragmentRes.isErr()) { + if (contentFragmentRes.error.code === "file_type_not_supported") { + return apiError(req, res, { + status_code: 400, + api_error: { + type: "invalid_request_error", + message: contentFragmentRes.error.message, + }, + }); + } throw new Error(contentFragmentRes.error.message); } - contentFragment = contentFragmentRes.value; } const contentFragmentRes = await postNewContentFragment( diff --git a/front/pages/api/v1/w/[wId]/assistant/conversations/index.ts b/front/pages/api/v1/w/[wId]/assistant/conversations/index.ts index 0577ff6f3d2a..e321ee30db62 100644 --- a/front/pages/api/v1/w/[wId]/assistant/conversations/index.ts +++ b/front/pages/api/v1/w/[wId]/assistant/conversations/index.ts @@ -177,6 +177,15 @@ async function handler( contentFragment, }); if (contentFragmentRes.isErr()) { + if (contentFragmentRes.error.code === "file_type_not_supported") { + return apiError(req, res, { + status_code: 400, + api_error: { + type: "invalid_request_error", + message: contentFragmentRes.error.message, + }, + }); + } throw new Error(contentFragmentRes.error.message); } contentFragment = contentFragmentRes.value; From 03441a735b6f4b648dc3ce40e8d8197cf4bf1a91 Mon Sep 17 00:00:00 2001 From: filou Date: Tue, 18 Mar 2025 16:06:12 +0100 Subject: [PATCH 2/3] cleanc --- .../w/[wId]/assistant/conversations/[cId]/content_fragments.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/front/pages/api/v1/w/[wId]/assistant/conversations/[cId]/content_fragments.ts b/front/pages/api/v1/w/[wId]/assistant/conversations/[cId]/content_fragments.ts index 8543d650ca99..7d42ac36a1d8 100644 --- a/front/pages/api/v1/w/[wId]/assistant/conversations/[cId]/content_fragments.ts +++ b/front/pages/api/v1/w/[wId]/assistant/conversations/[cId]/content_fragments.ts @@ -114,7 +114,7 @@ async function handler( } } const { context, ...rest } = r.data; - const contentFragment = rest; + let contentFragment = rest; // If we receive a content fragment that is not file based, we transform it to a file-based // one. @@ -134,6 +134,7 @@ async function handler( } throw new Error(contentFragmentRes.error.message); } + contentFragment = contentFragmentRes.value; } const contentFragmentRes = await postNewContentFragment( From 44ea4fd632a2cae778069c669e8e1826e117e8c8 Mon Sep 17 00:00:00 2001 From: filou Date: Tue, 18 Mar 2025 16:34:32 +0100 Subject: [PATCH 3/3] clean --- front/lib/api/assistant/conversation/content_fragment.ts | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/front/lib/api/assistant/conversation/content_fragment.ts b/front/lib/api/assistant/conversation/content_fragment.ts index 398974dc0bf0..5e898c09d885 100644 --- a/front/lib/api/assistant/conversation/content_fragment.ts +++ b/front/lib/api/assistant/conversation/content_fragment.ts @@ -1,8 +1,5 @@ -import type { - ProcessAndStoreFileError} from "@app/lib/api/files/upload"; -import { - processAndStoreFile -} from "@app/lib/api/files/upload"; +import type { ProcessAndStoreFileError } from "@app/lib/api/files/upload"; +import { processAndStoreFile } from "@app/lib/api/files/upload"; import type { Authenticator } from "@app/lib/auth"; import { FileResource } from "@app/lib/resources/file_resource"; import { getResourceIdFromSId } from "@app/lib/resources/string_ids";