Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve error handling for app extensions #17191

Merged
merged 15 commits into from Mar 16, 2023

Conversation

azrikahar
Copy link
Member

@azrikahar azrikahar commented Jan 16, 2023

Description

Closes ENG-56

Currently when an app extension is faulty or contains bugs, the error thrown will brick the app, thus requiring a browser refresh.

This PR aims to handle said errors more gracefully for app extensions such as panels, flows, interface and displays using the onErrorCaptured() lifecycle hook. It is then used inside a <v-error-boundary> component, similar to React's Error Boundaries and a Vue counterpart of error boundary.

For Module and Layout extensions, currently the added global error handler does prevent the app from breaking entirely anymore, but I've opted to not implement "low-level" error handler for them because they may incur more changes, to be more specific:

  • Module: After this PR, module that has error will be a blank route, but we can go back to previous page since the global error handler prevents it from crashing.

    The blocker for lower level module error handler would be each module is wrapped by the RouterPass here:

    for (const module of registeredModules.value) {
    router.addRoute({
    name: module.id,
    path: `/${module.id}`,
    component: RouterPass,
    children: module.routes,
    });
    }

    which is a functional component used in several other places:

    import { h, Component } from 'vue';
    import { RouterView } from 'vue-router';
    const component: Component = () => h(RouterView);
    component.displayName = 'router-passthrough';
    export default component;

    so if we want to narrow down and capture module errors "earlier", we may need to have another router view wrapper specifically for modules.

  • Layout: After this PR, layout that has error will just be empty (but nav, sidebar etc are all intact) and will not crash the app anymore.

    THe blocker for lower level layout error handler would be layouts are generally wrapped into LayoutWrapper in several places, so if we wish to capture layout specific errors, we may need to cover all layout wrapper usages in the app.

Panels

Before After
Breaks the app, requires refresh Shows an error icon and text, similar to when there's API errors when loading the panel(s)

Flows

Before After
Breaks the app, requires refresh Operation shows with error icon and text, similar to insights panels with errors

Interfaces

Before After
Breaks the app, requires refresh Shows a warning notice

Displays

  • render-display

    Before After
    Breaks the layout Fallbacks to the raw value in text form
  • render-template

    Before After
    Fallbacks to the raw value in text form

Type of Change

  • Bugfix
  • Improvement
  • New Feature
  • Refactor / codestyle updates
  • Other, please describe:

Requirements Checklist

  • New / updated tests are included
  • All tests are passing locally
  • Performed a self-review of the submitted code

If adding a new feature:

  • Documentation was added/updated. PR:

@azrikahar azrikahar marked this pull request as ready for review January 17, 2023 13:01
app/src/main.ts Show resolved Hide resolved
@azrikahar azrikahar requested review from a team, br41nslug, ConnorSimply, jaads and Nitwel and removed request for a team, br41nslug and jaads February 24, 2023 13:20
Copy link
Member

@ConnorSimply ConnorSimply left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, I didn't fully do extensive testing since it seemed like you did a lot with your gif's and the changes from Rijk above still needing to be decided/made.

Copy link
Member

@Nitwel Nitwel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good, nice addition!

@Nitwel Nitwel merged commit eb65d60 into main Mar 16, 2023
4 checks passed
@Nitwel Nitwel deleted the improve-app-extensions-error-handling branch March 16, 2023 12:04
@licitdev licitdev added this to the Next Release milestone Mar 16, 2023
meditadvisors pushed a commit to ciso360ai/directus-mod that referenced this pull request May 13, 2023
* add util function to get vue component name

* add global error handler

* add v-error-boundary component

* use error boundary to wrap insights panels

* use error boundary to wrap form interfaces

* use error boundary in render display and template

* use error boundary in extension options

* use error boundary for flows operation overview

* extract default options-overview into a component

* add tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants