Merge pull request #4344 from serlo/refactor/improve-error-handling-i…

…n-image-upload

refactor(image-plugin): Show better error messages when the image upload fails

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 {
@@ -76,66 +80,105 @@ export const createTestingImagePlugin = (secret: string) => {
 }
 
 function createUploadImageHandler(secret: string) {
-  const readFile = createReadFile(secret)
+  const readAndUploadFile = createReadAndUploadFile(secret)
   return async function uploadImageHandler(file: File): Promise<string> {
     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 readAndUploadFile(file)
+      return result.dataUrl
+    } catch (error) {
+      // eslint-disable-next-line no-console
+      console.error('Upload failed:', error)
+      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)
+    }
   }
 }
 
-export function createReadFile(secret: string) {
-  return async function readFile(file: File): Promise<LoadedFile> {
-    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)
+interface GraphQlResponse {
+  data: MediaUploadQuery | null
+  errors?: Array<{
+    message: string
+    extensions?: {
+      code?: string
+    }
+  }>
+}
+
+export function createReadAndUploadFile(secret: string) {
+  return async function readAndUploadFile(file: File): Promise<LoadedFile> {
+    if (!secret) {
+      throw new Error(FileErrorCode.SECRET_MISSING.toString())
+    }
+
+    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(FileErrorCode.NETWORK_ERROR.toString())
+    }
+
+    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(FileErrorCode.INVALID_RESPONSE.toString())
+    }
 
-      void runFetch()
+    const uploadResponse = await fetch(data.media.newUpload.uploadUrl, {
+      method: 'PUT',
+      headers: { 'Content-Type': file.type },
+      body: file,
     })
+
+    if (!uploadResponse.ok) {
+      throw new Error(FileErrorCode.UPLOAD_FAILED.toString())
+    }
+
+    return {
+      file,
+      dataUrl: data.media.newUpload.urlAfterUpload,
+    }
   }
 }
 
@@ -151,7 +194,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')
 }
 
@@ -167,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'
   }
 }
 

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)
 }