From 03e29b3fbcb639850c99d87f5cf1d9016e8dac7b Mon Sep 17 00:00:00 2001
From: wvffle <funkwhale@wvffle.net>
Date: Fri, 1 Jul 2022 11:13:07 +0000
Subject: [PATCH] Fix playlist modal error handling

---
 .../components/playlists/PlaylistModal.vue    |  14 ++-
 front/src/init/axios.ts                       | 103 ++++++++++--------
 front/src/types.ts                            |  10 +-
 front/src/utils/index.ts                      |  35 +++---
 4 files changed, 89 insertions(+), 73 deletions(-)

diff --git a/front/src/components/playlists/PlaylistModal.vue b/front/src/components/playlists/PlaylistModal.vue
index 055dfba5f..3f023f30e 100644
--- a/front/src/components/playlists/PlaylistModal.vue
+++ b/front/src/components/playlists/PlaylistModal.vue
@@ -1,7 +1,7 @@
 <script setup lang="ts">
 import { filter, sortBy, flow } from 'lodash-es'
 
-import axios, { AxiosError } from 'axios'
+import axios from 'axios'
 import { useGettext } from 'vue3-gettext'
 
 import Modal from '~/components/semantic/Modal.vue'
@@ -9,7 +9,7 @@ import PlaylistForm from '~/components/playlists/Form.vue'
 import useLogger from '~/composables/useLogger'
 import { useStore } from '~/store'
 import { ref, computed, watch } from 'vue'
-import { BackendError, Playlist } from '~/types'
+import { BackendError, Playlist, APIErrorResponse } from '~/types'
 import { useRouter } from 'vue-router'
 
 const logger = useLogger()
@@ -46,7 +46,7 @@ watch(() => store.state.playlists.showModal, () => {
 })
 
 const lastSelectedPlaylist = ref(-1)
-const errors = ref([] as AxiosError[])
+const errors = ref([] as string[])
 const duplicateTrackAddInfo = ref({} as { playlist_name?: string })
 
 const addToPlaylist = async (playlistId: number, allowDuplicates: boolean) => {
@@ -63,10 +63,12 @@ const addToPlaylist = async (playlistId: number, allowDuplicates: boolean) => {
     store.dispatch('playlists/fetchOwn')
   } catch (error) {
     if (error as BackendError) {
-      const { backendErrors } = error as BackendError
+      const { backendErrors, rawPayload = {} } = error as BackendError
 
-      if (backendErrors.length === 1 && backendErrors[0].code === 'tracks_already_exist_in_playlist') {
-        duplicateTrackAddInfo.value = backendErrors[0] as unknown as { playlist_name: string } 
+      // TODO (wvffle): Test if it works
+      // if (backendErrors.length === 1 && backendErrors[0].code === 'tracks_already_exist_in_playlist') {
+      if (backendErrors.length === 1 && backendErrors[0] === 'Tracks already exist in playlist') {
+        duplicateTrackAddInfo.value = ((rawPayload.playlist as APIErrorResponse).non_field_errors as APIErrorResponse)[0] as object
         showDuplicateTrackAddConfirmation.value = true
       } else {
         errors.value = backendErrors
diff --git a/front/src/init/axios.ts b/front/src/init/axios.ts
index e390e7731..9a3093d5d 100644
--- a/front/src/init/axios.ts
+++ b/front/src/init/axios.ts
@@ -1,4 +1,4 @@
-import { BackendError, InitModule, RateLimitStatus } from '~/types'
+import { APIErrorResponse, BackendError, InitModule, RateLimitStatus } from '~/types'
 
 import createAuthRefreshInterceptor from 'axios-auth-refresh'
 import axios, { AxiosError } from 'axios'
@@ -30,56 +30,71 @@ export const install: InitModule = ({ store, router }) => {
     return response
   }, async (error: BackendError) => {
     error.backendErrors = []
+
     if (store.state.auth.authenticated && !store.state.auth.oauth.accessToken && error.response?.status === 401) {
       store.commit('auth/authenticated', false)
       logger.warn('Received 401 response from API, redirecting to login form', router.currentRoute.value.fullPath)
       await router.push({ name: 'login', query: { next: router.currentRoute.value.fullPath } })
     }
 
-    if (error.response?.status === 404) {
-      error.backendErrors.push('Resource not found')
-      const message = error.response?.data
-      store.commit('ui/addMessage', {
-        content: message,
-        class: 'error'
-      })
-    } else if (error.response?.status === 403) {
-      error.backendErrors.push('Permission denied')
-    } else if (error.response?.status === 429) {
-      let message
-      const rateLimitStatus: RateLimitStatus = {
-        limit: error.response?.headers['x-ratelimit-limit'],
-        scope: error.response?.headers['x-ratelimit-scope'],
-        remaining: error.response?.headers['x-ratelimit-remaining'],
-        duration: error.response?.headers['x-ratelimit-duration'],
-        availableSeconds: parseInt(error.response?.headers['retry-after'] ?? 60),
-        reset: error.response?.headers['x-ratelimit-reset'],
-        resetSeconds: error.response?.headers['x-ratelimit-resetseconds']
-      }
-      if (rateLimitStatus.availableSeconds) {
-        const tryAgain = moment().add(rateLimitStatus.availableSeconds, 's').toNow(true)
-        message = $pgettext('*/Error/Paragraph', 'You sent too many requests and have been rate limited, please try again in %{ delay }')
-        message = $gettext(message, { delay: tryAgain })
-      } else {
-        message = $pgettext('*/Error/Paragraph', 'You sent too many requests and have been rate limited, please try again later')
-      }
-      error.backendErrors.push(message)
-      store.commit('ui/addMessage', {
-        content: message,
-        date: new Date(),
-        class: 'error'
-      })
-      logger.error('This client is rate-limited!', rateLimitStatus)
-    } else if (error.response?.status === 500) {
-      error.backendErrors.push('A server error occurred')
-    } else if (error.response?.data) {
-      if (error.response?.data.detail) {
-        error.backendErrors.push(error.response.data.detail)
-      } else {
-        error.rawPayload = error.response.data
-        const parsedErrors = parseAPIErrors(error.response.data)
-        error.backendErrors = [...error.backendErrors, ...parsedErrors]
+    switch (error.response?.status) {
+      case 404:
+        error.backendErrors.push('Resource not found')
+        store.commit('ui/addMessage', {
+          content: error.response?.data,
+          class: 'error'
+        })
+
+      case 403:
+        error.backendErrors.push('Permission denied')
+        break
+
+      case 429: {
+        let message
+        const rateLimitStatus: RateLimitStatus = {
+          limit: error.response?.headers['x-ratelimit-limit'],
+          scope: error.response?.headers['x-ratelimit-scope'],
+          remaining: error.response?.headers['x-ratelimit-remaining'],
+          duration: error.response?.headers['x-ratelimit-duration'],
+          availableSeconds: parseInt(error.response?.headers['retry-after'] ?? 60),
+          reset: error.response?.headers['x-ratelimit-reset'],
+          resetSeconds: error.response?.headers['x-ratelimit-resetseconds']
+        }
+
+        if (rateLimitStatus.availableSeconds) {
+          const tryAgain = moment().add(rateLimitStatus.availableSeconds, 's').toNow(true)
+          message = $pgettext('*/Error/Paragraph', 'You sent too many requests and have been rate limited, please try again in %{ delay }')
+          message = $gettext(message, { delay: tryAgain })
+        } else {
+          message = $pgettext('*/Error/Paragraph', 'You sent too many requests and have been rate limited, please try again later')
+        }
+
+        error.backendErrors.push(message)
+        store.commit('ui/addMessage', {
+          content: message,
+          date: new Date(),
+          class: 'error'
+        })
+
+        logger.error('This client is rate-limited!', rateLimitStatus)
+        break
       }
+
+      case 500:
+        error.backendErrors.push('A server error occurred')
+        break
+      
+      default:
+        if (error.response?.data as object) {
+          const data = error.response?.data as Record<string, unknown>
+          if (data?.detail) {
+            error.backendErrors.push(data.detail as string)
+          } else {
+            error.rawPayload = data as APIErrorResponse
+            const parsedErrors = parseAPIErrors(data as APIErrorResponse)
+            error.backendErrors = [...error.backendErrors, ...parsedErrors]
+          }
+        }
     }
 
     if (error.backendErrors.length === 0) {
diff --git a/front/src/types.ts b/front/src/types.ts
index 43aa2145f..eb868a1c5 100644
--- a/front/src/types.ts
+++ b/front/src/types.ts
@@ -94,13 +94,11 @@ export interface Playlist {
 }
 
 // API stuff
-export interface APIErrorResponse {
-  [key: string]: APIErrorResponse | string[]
-}
+export interface APIErrorResponse extends Record<string, APIErrorResponse | string[] | { code: string }[]> {}
 
-export interface BackendError {
-  backendErrors: AxiosError[]
-  rawPayload?: object
+export interface BackendError extends AxiosError {
+  backendErrors: string[]
+  rawPayload?: APIErrorResponse
 }
 
 export interface RateLimitStatus {
diff --git a/front/src/utils/index.ts b/front/src/utils/index.ts
index 031142e9e..e1c354cf4 100644
--- a/front/src/utils/index.ts
+++ b/front/src/utils/index.ts
@@ -11,27 +11,28 @@ export function setUpdate (obj: object, statuses: Record<string, unknown>, value
 
 export function parseAPIErrors (responseData: APIErrorResponse, parentField?: string): string[] {
   const errors = []
-  for (const field in responseData) {
-    if (Object.prototype.hasOwnProperty.call(responseData, field)) {
-      let fieldName = startCase(field.replace('_', ' '))
-      if (parentField) {
-        fieldName = `${parentField} - ${fieldName}`
-      }
+  for (const [field, value] of Object.entries(responseData)) {
+    let fieldName = startCase(field.replace(/_/g, ' '))
+    if (parentField) {
+      fieldName = `${parentField} - ${fieldName}`
+    }
 
-      const value = responseData[field]
-      if (Array.isArray(value)) {
-        const values = value
-        errors.push(...values.map(err => {
+    if (Array.isArray(value)) {
+      errors.push(...value.map(err => {
+        if (typeof err === 'string') {
           return err.toLocaleLowerCase().includes('this field ')
             ? `${fieldName}: ${err}`
             : err
-        }))
-      } else if (value) {
-        // nested errors
-        const nestedErrors = parseAPIErrors(value, fieldName)
-        errors.push(...nestedErrors)
-      }
-    }
+        }
+
+        return startCase(err.code.replace(/_/g, ' '))
+      }))
+
+      continue
+    } 
+
+    // Handle nested errors
+    errors.push(...parseAPIErrors(value, fieldName))
   }
 
   return errors