From cf67310fea6eb1724f4144ae403113a223d358a6 Mon Sep 17 00:00:00 2001 From: Mikey Stengel Date: Tue, 10 Dec 2024 11:54:50 +0100 Subject: [PATCH 1/5] refactor(image-plugin): Throw more errors, show more toasts in old upload --- .../src/plugins/image/utils/upload-file.ts | 21 ++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/packages/editor/src/plugins/image/utils/upload-file.ts b/packages/editor/src/plugins/image/utils/upload-file.ts index d75a114107..4737873f74 100644 --- a/packages/editor/src/plugins/image/utils/upload-file.ts +++ b/packages/editor/src/plugins/image/utils/upload-file.ts @@ -60,16 +60,31 @@ async function uploadFile({ handleError(errorMessage) }) - const data = (await result?.json()) as { + if (result && !result.ok) { + const error = new Error('Failed to get signed URL') + handleError(error.message) + return Promise.reject(error) + } + + const data = (await result?.json().catch(() => null)) as { signedUrl: string fileUrl: string + } | null + if (!data) { + const error = new Error('Failed to get signed URL') + handleError(error.message) + + return Promise.reject(error) } - if (!data) return Promise.reject(new Error('Could not get signed URL')) const { signedUrl, fileUrl } = data const success = await uploadToBucket({ file, signedUrl }) - if (!success) return Promise.reject(new Error('Could not upload file')) + if (!success) { + const error = new Error('Failed to upload file') + handleError(error.message) + return Promise.reject(error) + } return Promise.resolve(fileUrl) } From 0599ec189d93673381f18822a89f64383cc9e79d Mon Sep 17 00:00:00 2001 From: Mikey Stengel Date: Tue, 10 Dec 2024 15:40:31 +0100 Subject: [PATCH 2/5] refactor(image): Simplify code and show toast in old upload --- .../image-with-testing-config.ts | 107 ++++++++++-------- 1 file changed, 61 insertions(+), 46 deletions(-) diff --git a/packages/editor/src/editor-integration/image-with-testing-config.ts b/packages/editor/src/editor-integration/image-with-testing-config.ts index da9f1a7cb2..da9d5efde3 100644 --- a/packages/editor/src/editor-integration/image-with-testing-config.ts +++ b/packages/editor/src/editor-integration/image-with-testing-config.ts @@ -80,62 +80,77 @@ function createUploadImageHandler(secret: string) { return async function uploadImageHandler(file: File): Promise { const validation = validateFile(file) if (!validation.valid) { - onError(validation.errors) + showErrorToast(validation.errors) // eslint-disable-next-line @typescript-eslint/prefer-promise-reject-errors return Promise.reject(validation.errors) } - return (await readFile(file)).dataUrl + try { + const result = await readFile(file) + return result.dataUrl + } catch (error) { + // eslint-disable-next-line no-console + console.error('Upload failed:', error) + const errors = handleErrors([FileErrorCode.UPLOAD_FAILED]) + showErrorToast(errors) + // eslint-disable-next-line @typescript-eslint/prefer-promise-reject-errors + return Promise.reject(errors) + } } } export function createReadFile(secret: string) { return async function readFile(file: File): Promise { - return new Promise((resolve, reject) => { - async function runFetch() { - const endpoint = 'https://api.serlo-staging.dev/graphql' - const response = await fetch(endpoint, { - headers: { - Accept: 'application/json', - 'Content-Type': 'application/json', - 'X-SERLO-EDITOR-TESTING': secret, - }, - method: 'POST', - body: JSON.stringify({ - query: uploadUrlQuery, - variables: { - mediaType: mimeTypesToMediaType[file.type as SupportedMimeType], - }, - }), - }) - const { data } = (await response.json()) as { data: MediaUploadQuery } - const reader = new FileReader() - - reader.onload = async function (e: ProgressEvent) { - if (!e.target) return - - try { - const response = await fetch(data.media.newUpload.uploadUrl, { - method: 'PUT', - headers: { 'Content-Type': file.type }, - body: file, - }) - - if (response.status !== 200) reject() - resolve({ - file, - dataUrl: data.media.newUpload.urlAfterUpload, - }) - } catch { - reject() - } - } - - reader.readAsDataURL(file) - } + if (!secret) { + throw new Error('Missing secret for image plugin!') + } - void runFetch() + const endpoint = 'https://api.serlo-staging.dev/graphql' + const response = await fetch(endpoint, { + headers: { + Accept: 'application/json', + 'Content-Type': 'application/json', + 'X-SERLO-EDITOR-TESTING': secret, + }, + method: 'POST', + body: JSON.stringify({ + query: uploadUrlQuery, + variables: { + mediaType: mimeTypesToMediaType[file.type as SupportedMimeType], + }, + }), }) + + if (!response.ok) { + throw new Error(`Failed to get upload URL: ${response.status}`) + } + + const { data } = (await response.json()) as { data: MediaUploadQuery } + + if (!data?.media?.newUpload) { + // eslint-disable-next-line no-console + console.error('Server responded with following invalid data: ', data) + throw new Error('Invalid response format from server') + } + + const uploadResponse = await fetch(data.media.newUpload.uploadUrl, { + method: 'PUT', + headers: { 'Content-Type': file.type }, + body: file, + }) + + if (!uploadResponse.ok) { + throw new Error(`Upload failed with status: ${uploadResponse.status}`) + } + + if (!data?.media?.newUpload?.urlAfterUpload) { + throw new Error('Invalid response format from server') + } + + return { + file, + dataUrl: data.media.newUpload.urlAfterUpload, + } } } @@ -151,7 +166,7 @@ function handleErrors(errors: FileErrorCode[]): FileError[] { })) } -function onError(errors: FileError[]): void { +function showErrorToast(errors: FileError[]): void { showToastNotice(errors.map((error) => error.message).join('\n'), 'warning') } From 17580936770caab936962034722799289f86b557 Mon Sep 17 00:00:00 2001 From: Mikey Stengel Date: Tue, 10 Dec 2024 15:42:48 +0100 Subject: [PATCH 3/5] refactor(image): Rename readFile fn to readAndUploadFile --- .../src/editor-integration/image-with-testing-config.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/editor/src/editor-integration/image-with-testing-config.ts b/packages/editor/src/editor-integration/image-with-testing-config.ts index da9d5efde3..b1caba51b9 100644 --- a/packages/editor/src/editor-integration/image-with-testing-config.ts +++ b/packages/editor/src/editor-integration/image-with-testing-config.ts @@ -76,7 +76,7 @@ export const createTestingImagePlugin = (secret: string) => { } function createUploadImageHandler(secret: string) { - const readFile = createReadFile(secret) + const readAndUploadFile = createReadAndUploadFile(secret) return async function uploadImageHandler(file: File): Promise { const validation = validateFile(file) if (!validation.valid) { @@ -86,7 +86,7 @@ function createUploadImageHandler(secret: string) { } try { - const result = await readFile(file) + const result = await readAndUploadFile(file) return result.dataUrl } catch (error) { // eslint-disable-next-line no-console @@ -99,8 +99,8 @@ function createUploadImageHandler(secret: string) { } } -export function createReadFile(secret: string) { - return async function readFile(file: File): Promise { +export function createReadAndUploadFile(secret: string) { + return async function readAndUploadFile(file: File): Promise { if (!secret) { throw new Error('Missing secret for image plugin!') } From 45c0d35e7a1ebe44cb0eaa4eb2ecbea2994cb4e3 Mon Sep 17 00:00:00 2001 From: Mikey Stengel Date: Tue, 10 Dec 2024 15:50:39 +0100 Subject: [PATCH 4/5] refactor(image): Streamline error handling --- .../src/editor-integration/image-with-testing-config.ts | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/packages/editor/src/editor-integration/image-with-testing-config.ts b/packages/editor/src/editor-integration/image-with-testing-config.ts index b1caba51b9..35e8c0aa7a 100644 --- a/packages/editor/src/editor-integration/image-with-testing-config.ts +++ b/packages/editor/src/editor-integration/image-with-testing-config.ts @@ -127,7 +127,10 @@ export function createReadAndUploadFile(secret: string) { const { data } = (await response.json()) as { data: MediaUploadQuery } - if (!data?.media?.newUpload) { + if ( + !data?.media?.newUpload?.uploadUrl || + !data.media.newUpload.urlAfterUpload + ) { // eslint-disable-next-line no-console console.error('Server responded with following invalid data: ', data) throw new Error('Invalid response format from server') @@ -143,10 +146,6 @@ export function createReadAndUploadFile(secret: string) { throw new Error(`Upload failed with status: ${uploadResponse.status}`) } - if (!data?.media?.newUpload?.urlAfterUpload) { - throw new Error('Invalid response format from server') - } - return { file, dataUrl: data.media.newUpload.urlAfterUpload, From fd09dcff1ed01390120721d7911b99bdb1b637ea Mon Sep 17 00:00:00 2001 From: Mikey Stengel Date: Tue, 10 Dec 2024 16:27:54 +0100 Subject: [PATCH 5/5] refactor(image): Add more error codes and display exact error instead of just generic "upload failed" --- .../image-with-testing-config.ts | 49 ++++++++++++++++--- 1 file changed, 43 insertions(+), 6 deletions(-) diff --git a/packages/editor/src/editor-integration/image-with-testing-config.ts b/packages/editor/src/editor-integration/image-with-testing-config.ts index 35e8c0aa7a..bc14f327f7 100644 --- a/packages/editor/src/editor-integration/image-with-testing-config.ts +++ b/packages/editor/src/editor-integration/image-with-testing-config.ts @@ -45,6 +45,10 @@ enum FileErrorCode { BAD_EXTENSION, FILE_TOO_BIG, UPLOAD_FAILED, + UNAUTHORIZED, + SECRET_MISSING, + INVALID_RESPONSE, + NETWORK_ERROR, } export interface FileError { @@ -91,7 +95,12 @@ function createUploadImageHandler(secret: string) { } catch (error) { // eslint-disable-next-line no-console console.error('Upload failed:', error) - const errors = handleErrors([FileErrorCode.UPLOAD_FAILED]) + const errorCode = + error instanceof Error + ? Number(error.message) || FileErrorCode.UPLOAD_FAILED + : FileErrorCode.UPLOAD_FAILED + + const errors = handleErrors([errorCode]) showErrorToast(errors) // eslint-disable-next-line @typescript-eslint/prefer-promise-reject-errors return Promise.reject(errors) @@ -99,10 +108,20 @@ function createUploadImageHandler(secret: string) { } } +interface GraphQlResponse { + data: MediaUploadQuery | null + errors?: Array<{ + message: string + extensions?: { + code?: string + } + }> +} + export function createReadAndUploadFile(secret: string) { return async function readAndUploadFile(file: File): Promise { if (!secret) { - throw new Error('Missing secret for image plugin!') + throw new Error(FileErrorCode.SECRET_MISSING.toString()) } const endpoint = 'https://api.serlo-staging.dev/graphql' @@ -122,18 +141,28 @@ export function createReadAndUploadFile(secret: string) { }) if (!response.ok) { - throw new Error(`Failed to get upload URL: ${response.status}`) + throw new Error(FileErrorCode.NETWORK_ERROR.toString()) } - const { data } = (await response.json()) as { data: MediaUploadQuery } + const { data, errors } = (await response.json()) as GraphQlResponse + + if (errors?.length) { + // eslint-disable-next-line no-console + console.error('GraphQL errors:', errors) + if (errors[0]?.extensions?.code === 'UNAUTHENTICATED') { + throw new Error(FileErrorCode.UNAUTHORIZED.toString()) + } + throw new Error(FileErrorCode.UPLOAD_FAILED.toString()) + } if ( + !data || !data?.media?.newUpload?.uploadUrl || !data.media.newUpload.urlAfterUpload ) { // eslint-disable-next-line no-console console.error('Server responded with following invalid data: ', data) - throw new Error('Invalid response format from server') + throw new Error(FileErrorCode.INVALID_RESPONSE.toString()) } const uploadResponse = await fetch(data.media.newUpload.uploadUrl, { @@ -143,7 +172,7 @@ export function createReadAndUploadFile(secret: string) { }) if (!uploadResponse.ok) { - throw new Error(`Upload failed with status: ${uploadResponse.status}`) + throw new Error(FileErrorCode.UPLOAD_FAILED.toString()) } return { @@ -181,6 +210,14 @@ function errorCodeToMessage(error: FileErrorCode) { return 'Filesize is too big' case FileErrorCode.UPLOAD_FAILED: return 'Error while uploading' + case FileErrorCode.UNAUTHORIZED: + return 'You are not authorized to upload images. Ensure the testingSecret is correct!' + case FileErrorCode.SECRET_MISSING: + return 'Missing authentication credentials (testingSecret)!' + case FileErrorCode.INVALID_RESPONSE: + return 'Server returned invalid data' + case FileErrorCode.NETWORK_ERROR: + return 'Network error while uploading' } }