Rework error handling

allow full control of error handling with global fallback handler
add 404 error
add websocket opening control for get methods

app/src/api/api.js

@@ -51,20 +51,23 @@ export default {
    *
    * @param {string} method - a method between 'GET', 'POST', 'PUT' and 'DELETE'.
    * @param {string} uri
-   * @param {string} [data={}] - data to send as body for 'POST', 'PUT' and 'DELETE' methods.
+   * @param {Object} [data={}] - data to send as body for 'POST', 'PUT' and 'DELETE' methods.
+   * @param {Object} [options={}]
+   * @param {Boolean} [options.websocket=true] - Open a websocket before this request.
    * @return {Promise<Response>} Promise that resolve a fetch `Response`.
    */
-  async fetch (method, uri, data = {}) {
+  async fetch (method, uri, data = {}, { websocket = true } = {}) {
     // Open a websocket connection that will dispatch messages received.
-    // FIXME add ability to do not open it
+    if (websocket) {
       await this.openWebSocket()
+      store.dispatch('WAITING_FOR_RESPONSE', [uri, method])
+    }
 
     if (method === 'GET') {
       const localeQs = `${uri.includes('?') ? '&' : '?'}locale=${store.getters.locale}`
       return fetch('/yunohost/api/' + uri + localeQs, this.options)
     }
 
-    store.dispatch('WAITING_FOR_RESPONSE', [uri, method])
     return fetch('/yunohost/api/' + uri, {
       ...this.options,
       method,
@@ -76,10 +79,12 @@ export default {
    * Api get helper function.
    *
    * @param {string} uri - the uri to call.
+   * @param {Object} [options={}]
+   * @param {Boolean} [options.websocket=false] - Open a websocket before this request.
    * @return {Promise<module:api~DigestedResponse>} Promise that resolve the api response as an object, a string or as an error.
    */
-  get (uri) {
+  get (uri, { websocket = false } = {}) {
-    return this.fetch('GET', uri).then(response => handleResponse(response, 'GET'))
+    return this.fetch('GET', uri, null, { websocket }).then(response => handleResponse(response, 'GET'))
   },
 
   /**

app/src/api/errors.js

@@ -5,82 +5,100 @@
 
 import i18n from '@/i18n'
 
+
 class APIError extends Error {
-  constructor (method, { url, status, statusText }, message) {
+  constructor (method, { url, status, statusText }, errorData) {
-    super(message || i18n.t('error_server_unexpected'))
+    super(errorData.error || i18n.t('error_server_unexpected'))
-    this.uri = new URL(url).pathname.replace('/yunohost', '')
+    const urlObj = new URL(url)
-    this.method = method
+    this.name = 'APIError'
     this.code = status
     this.status = statusText
-    this.name = 'APIError'
+    this.method = method
+    this.path = urlObj.pathname + urlObj.search
+    this.logRef = errorData.log_ref || null
   }
 
-  print () {
+  log () {
     console.error(`${this.name} (${this.code}): ${this.uri}\n${this.message}`)
   }
 }
 
-// 401 — Unauthorized
-class APIUnauthorizedError extends APIError {
-  constructor (method, response, message) {
-    super(method, response, i18n.t('unauthorized'))
-    this.name = 'APIUnauthorizedError'
-  }
-}
-
-// 400 — Bad Request
-class APIBadRequestError extends APIError {
-  constructor (method, response, message) {
-    super(method, response, message)
-    this.name = 'APIBadRequestError'
-  }
-}
-
-// 500 — Server Internal Error
-class APIInternalError extends APIError {
-  constructor (method, response, data) {
-    // not tested (message should be json but in )
-    const traceback = typeof data === 'object' ? data.traceback : null
-    super(method, response, 'none')
-    if (traceback) {
-      this.traceback = traceback
-    }
-    this.name = 'APIInternalError'
-  }
-}
-
-// 502 — Bad gateway (means API is down)
-class APINotRespondingError extends APIError {
-  constructor (method, response) {
-    super(method, response, i18n.t('api_not_responding'))
-    this.name = 'APINotRespondingError'
-  }
-}
 
 // 0 — (means "the connexion has been closed" apparently)
 class APIConnexionError extends APIError {
   constructor (method, response) {
-    super(method, response, i18n.t('error_connection_interrupted'))
+    super(method, response, { error: i18n.t('error_connection_interrupted') })
     this.name = 'APIConnexionError'
   }
 }
 
+
+// 400 — Bad Request
+class APIBadRequestError extends APIError {
+  constructor (method, response, errorData) {
+    super(method, response, errorData)
+    this.name = 'APIBadRequestError'
+  }
+}
+
+
+// 401 — Unauthorized
+class APIUnauthorizedError extends APIError {
+  constructor (method, response, errorData) {
+    super(method, response, { error: i18n.t('unauthorized') })
+    this.name = 'APIUnauthorizedError'
+  }
+}
+
+
+// 404 — Not Found
+class APINotFoundError extends APIError {
+  constructor (method, response, errorData) {
+    errorData.error = i18n.t('api_not_found')
+    super(method, response, errorData)
+    this.name = 'APINotFoundError'
+  }
+}
+
+
+// 500 — Server Internal Error
+class APIInternalError extends APIError {
+  constructor (method, response, errorData) {
+    super(method, response, errorData)
+    this.traceback = errorData.traceback || null
+    this.name = 'APIInternalError'
+  }
+}
+
+
+// 502 — Bad gateway (means API is down)
+class APINotRespondingError extends APIError {
+  constructor (method, response) {
+    super(method, response, { error: i18n.t('api_not_responding') })
+    this.name = 'APINotRespondingError'
+  }
+}
+
+
 // Temp factory
 const errors = {
   [undefined]: APIError,
   0: APIConnexionError,
   400: APIBadRequestError,
   401: APIUnauthorizedError,
+  404: APINotFoundError,
   500: APIInternalError,
   502: APINotRespondingError
 }
 
+
 export {
   errors as default,
   APIError,
-  APIUnauthorizedError,
   APIBadRequestError,
+  APIConnexionError,
   APIInternalError,
+  APINotFoundError,
   APINotRespondingError,
-  APIConnexionError
+  APIUnauthorizedError
 }

app/src/api/handlers.js

@@ -4,8 +4,8 @@
  */
 
 import store from '@/store'
-import errors from './errors'
+import errors, { APIError } from './errors'
-import router from '@/router'
+
 
 /**
  * Try to get response content as json and if it's not as text.
@@ -13,8 +13,7 @@ import router from '@/router'
  * @param {Response} response - A fetch `Response` object.
  * @return {(Object|String)} Parsed response's json or response's text.
  */
-
+async function _getResponseData (response) {
-async function _getResponseContent (response) {
   // FIXME the api should always return json as response
   const responseText = await response.text()
   try {
@@ -24,43 +23,64 @@ async function _getResponseContent (response) {
   }
 }
 
+
 /**
  * Handler for API responses.
  *
  * @param {Response} response - A fetch `Response` object.
  * @return {(Object|String)} Parsed response's json, response's text or an error.
  */
-export function handleResponse (response, method) {
+export async function handleResponse (response, method) {
-  if (method !== 'GET') {
+  const responseData = await _getResponseData(response)
-    store.dispatch('SERVER_RESPONDED', response.ok)
+  store.dispatch('SERVER_RESPONDED')
-  }
+  return response.ok ? responseData : handleError(response, responseData, method)
-  if (!response.ok) return handleError(response, method)
-  // FIXME the api should always return json objects
-  return _getResponseContent(response)
 }
 
+
 /**
  * Handler for API errors.
  *
  * @param {Response} response - A fetch `Response` object.
  * @throws Will throw a custom error with response data.
  */
-export async function handleError (response, method) {
+export async function handleError (response, errorData, method) {
-  const message = await _getResponseContent(response)
   const errorCode = response.status in errors ? response.status : undefined
-  const error = new errors[errorCode](method, response, message.error || message)
+  // FIXME API: Patching errors that are plain text or html.
-
+  if (typeof errorData === 'string') {
-  if (error.code === 401) {
+    errorData = { error: errorData }
-    store.dispatch('DISCONNECT')
-  } else if (error.code === 400) {
-    if (typeof message !== 'string' && 'log_ref' in message) {
-      router.push({ name: 'tool-log', params: { name: message.log_ref } })
-    }
-    // Hide the waiting screen
-    store.dispatch('SERVER_RESPONDED', true)
-  } else {
-    store.dispatch('DISPATCH_ERROR', error)
   }
 
-  throw error
+  // This error can be catched by a view otherwise it will be catched by the `onUnhandledAPIError` handler.
+  throw new errors[errorCode](method, response, errorData)
+}
+
+
+export function onUnhandledAPIError (error) {
+  // In 'development', Babel seems to also catch the error so there's no need to log it twice.
+  if (process.env.NODE_ENV !== 'development') {
+    error.log()
+  }
+  store.dispatch('HANDLE_ERROR', error)
+}
+
+
+export function registerGlobalErrorHandlers () {
+  // Global catching of unhandled promise's rejections.
+  // Those errors (thrown or rejected from inside a promise) can't be catched by `window.onerror`.
+  window.addEventListener('unhandledrejection', e => {
+    const error = e.reason
+    if (error instanceof APIError) {
+      onUnhandledAPIError(error)
+      // Seems like there's a bug in Firefox and the error logging in not prevented.
+      e.preventDefault()
+    }
+  })
+
+  // Keeping this in case it is needed.
+
+  // Global catching of errors occuring inside vue components.
+  // Vue.config.errorHandler = (err, vm, info) => {}
+
+  // Global catching of regular js errors.
+  // window.onerror = (message, source, lineno, colno, error) => {}
 }

app/src/api/index.js

@@ -1,2 +1,2 @@
 export { default } from './api'
-export { handleResponse, handleError } from './handlers'
+export { handleResponse, handleError, registerGlobalErrorHandlers } from './handlers'

app/src/components/ApiWaitOverlay.vue

@@ -2,13 +2,13 @@
   <b-overlay
     variant="white" rounded="sm" opacity="0.5"
     no-center
-    :show="waiting"
+    :show="show"
   >
     <slot name="default" />
 
     <template v-slot:overlay>
       <b-card no-body>
-        <div v-if="!error" class="mt-3 px-3">
+        <div v-if="error === null" class="mt-3 px-3">
           <div class="custom-spinner" :class="spinner" />
         </div>
 
@@ -31,7 +31,7 @@
         </b-card-body>
 
         <b-card-footer v-if="error" class="justify-content-end">
-          <b-button variant="primary" v-t="'ok'" @click="$store.dispatch('SERVER_RESPONDED', true)" />
+          <b-button variant="primary" v-t="'ok'" @click="$store.dispatch('DELETE_ERROR')" />
         </b-card-footer>
       </b-card>
     </template>
@@ -48,6 +48,10 @@ export default {
   computed: {
     ...mapGetters(['waiting', 'lastAction', 'error', 'spinner']),
 
+    show () {
+      return this.waiting || this.error !== null
+    },
+
     progress () {
       if (!this.lastAction) return null
       const progress = this.lastAction.progress

app/src/i18n/locales/en.json

@@ -22,10 +22,12 @@
         "APIError": "Yunohost encountered an unexpected error",
         "APIBadRequestError": "Yunohost encountered an error",
         "APIInternalError": "Yunohost encountered an internal error",
+        "APINotFoundError": "Yunohost API could not find a route",
         "APINotRespondingError": "Yunohost API is not responding",
-        "APIConnexionError": "Yunohost encountered an connexion error"
+        "APIConnexionError": "Yunohost encountered a connexion error"
     },
     "all_apps": "All apps",
+    "api_not_found": "Seems like the web-admin tryed to query something that doesn't exist.",
     "api_not_responding": "The YunoHost API is not responding. Maybe 'yunohost-api' is down or got restarted?",
     "api_waiting": "Waiting for the server's response...",
     "app_actions": "Actions",
@@ -189,9 +191,10 @@
         "title": "History",
         "last_action": "Last action:",
         "methods": {
+            "DELETE": "delete",
+            "GET": "read",
             "POST": "create/execute",
-            "PUT": "modify",
+            "PUT": "modify"
-            "DELETE": "delete"
         }
     },
     "home": "Home",

app/src/main.js

@@ -6,8 +6,12 @@ import i18n from './i18n'
 import router from './router'
 import store from './store'
 
+import { registerGlobalErrorHandlers } from './api'
+
+
 Vue.config.productionTip = false
 
+
 // Styles are imported in `src/App.vue` <style>
 Vue.use(BootstrapVue, {
   BSkeleton: { animation: 'none' },
@@ -20,6 +24,7 @@ Vue.use(BootstrapVue, {
   }
 })
 
+
 // Ugly wrapper for `$bvModal.msgBoxConfirm` to set default i18n button titles
 // FIXME find or wait for a better way
 Vue.prototype.$askConfirmation = function (message, props) {
@@ -30,6 +35,7 @@ Vue.prototype.$askConfirmation = function (message, props) {
   })
 }
 
+
 // Register global components
 const requireComponent = require.context('@/components/globals', true, /\.(js|vue)$/i)
 // For each matching file name...
@@ -40,6 +46,10 @@ requireComponent.keys().forEach((fileName) => {
   Vue.component(component.name, component)
 })
 
+
+registerGlobalErrorHandlers()
+
+
 new Vue({
   i18n,
   router,

app/src/store/info.js

@@ -109,11 +109,8 @@ export default {
       commit('ADD_HISTORY_ENTRY', [uri, method, Date.now()])
     },
 
-    'SERVER_RESPONDED' ({ state, dispatch, commit }, responseIsOk) {
+    'SERVER_RESPONDED' ({ commit }) {
-      if (responseIsOk) {
       commit('UPDATE_WAITING', false)
-        commit('SET_ERROR', '')
-      }
     },
 
     'DISPATCH_MESSAGE' ({ commit }, messages) {
@@ -139,12 +136,21 @@ export default {
       }
     },
 
-    'DISPATCH_ERROR' ({ state, commit }, error) {
+    'HANDLE_ERROR' ({ state, commit, dispatch }, error) {
+      if (error.code === 401) {
+        // Unauthorized
+        dispatch('DISCONNECT')
+      } else if (error.logRef) {
+        // Errors that have produced logs
+        router.push({ name: 'tool-log', params: { name: error.logRef } })
+      } else {
+        // Display the error in a modal on the current view.
         commit('SET_ERROR', error)
-      if (error.method === 'GET') {
-        router.push({ name: 'error', params: { type: error.code } })
       }
-      // else the waiting screen will display the error
+    },
+
+    'DELETE_ERROR' ({ commit }) {
+      commit('SET_ERROR', null)
     }
   },
 

app/src/views/ErrorPage.vue

@@ -13,7 +13,7 @@
     <pre><code>"{{ error.code }}" {{ error.status }}</code></pre>
 
     <h5 v-t="'action'" />
-    <pre><code>"{{ error.method }}" {{ error.uri }}</code></pre>
+    <pre><code>"{{ error.method }}" {{ error.path }}</code></pre>
 
     <h5>Message</h5>
     <p v-html="error.message" />