globalbrain / sefirot

Global Brain Design System.
https://sefirot.globalbrains.com
MIT License
150 stars 12 forks source link

[Error] Add unified error handling feature #476

Closed kiaking closed 8 months ago

kiaking commented 9 months ago

Add error handling that works with Sentry. Maybe we need different module for Nuxt?

Currently, every app is doing slightly different things when catching and reporting global errors. Would be nice to have more unified way to setup.

Idea

In Vue, error can happen on many different phases, but at the moment, we only care about error happening when running the Vue app. We can ignore error happening outside of Vue, for example css file is not loaded due to network request kinda things.

Next, inside Vue app, we usually only care catching errors inside "page". In reality, error could happen at almost anywhere. Like app.use() can throw error if the plugin is messed up. But those errors are very rare and usually caught during local dev, at least for now, we can ignore it.

So, ideally, I think having this kind of setup would make sense inside layout file.

<script setup lang="ts">
import { useErrorHandling } from 'sefirot/composables/Error'
import { useMe } from '@/composables/Auth'

// Get signed in user.
const me = useMe()

// Handle all app errors. Use `onErrorCaptured` inside.
// We could use `<SErrorBoundary>` as alternative.
//
// Make it able to pass in authenticated user info so that
// we can attach it when reporting to sentry.
const { error } = useErrorHandling(me)
</script>

<template>
  <div class="LayoutMain">
    <LayoutMainHeader />

    <div class="main">
      <!-- Show error page when there is a global error -->
      <ErrorPage v-if="error" :error="error" />
      <RouterView v-else />
    </div>

    <LayoutMainFooter />
  </div>
</template>

The useErrorHandling composable should catch all errors and report to Sentry. How sentry is integrated should also be thought, but this is one of useErrorHandling composable we have in our app.

/**
 * Capture an exception and send it to Sentry, and show error page.
 */
export function useError(): (err: Error) => void {
  return (err) => {
    config.app.env === 'local'
      ? console.error(err)
      : Sentry.captureException(err)
  }
}

export function useErrorHandling(): ErrorHandling {
  const createError = useError()

  const error = shallowRef<Error | null>(null)

  onErrorCaptured(handleError)

  // Setup `unhandledrejection` in order to catch error thrown inside of
  // promises since `onErrorCaptured` hook will not catch them.
  onMounted(() => {
    window.addEventListener('unhandledrejection', handleRejection)
  })

  onUnmounted(() => {
    window.removeEventListener('unhandledrejection', handleRejection)
  })

  function handleError(err: Error): false | void {
    error.value = err

    if (!(err instanceof PageNotFoundError)) {
      createError(err)
    }

    return false
  }

  function handleRejection(event: PromiseRejectionEvent): void {
    event.preventDefault()
    handleError(event.reason)
  }

  return {
    error
  }
}
nozomuikuta commented 9 months ago

Thank you for summarizing! Generally LGTM.

One thing is that, it's more intuitive to use useError() for getting the error itself or its wrapper object (cf. Nuxt's useError()). Seeing what it does, useErrorReporter() or useErrorCapturer would match.

const reportError = useErrorReporter()

reportError(err)
const captureError = useErrorCapturer()

captureError(err)

Another thing is, it's good to unify how to reset the error or restore the page among our apps. For example, Deus has restore button to reload the whole page while Fatima just shows some message. Maybe SErrorPage, I'm not so sure, though (cf. SPage that we talked about before).

Last, would you elaborate the implementation of SErrorBoundary?

We could use <SErrorBoundary> as alternative.

brc-dd commented 9 months ago

would you elaborate the implementation of SErrorBoundary?

https://github.com/globalbrain/sefirot/blob/main/lib/components/SErrorBoundary.vue -- it's same as Nuxt's one.

PS: Let's not use useError as name. It might cause conflicts with Nuxt.

nozomuikuta commented 9 months ago

Ah, SErrorBoundary is already released. 🙌 I couldn't catch up on all details of Sefirot releases these days.

Well, it looks exactly like SErrapPage that I referred to in the last comment. 👍