Frontend Error Handling #667

kschiffer · 2019-05-09T15:34:16Z

Summary

This PR adds a start for handling errors in the console / oauth app.
Closes #41
Closes #659

Changes

Extend the error utils
Add an error boundary component
Add top-most error boundary to the console app (to catch any unhandled exceptions)
Add error boundaries to application api key and collaborators management
Add a full view error component to display (critical) errors
Add a sub view error component to display (non-critical) errors inside sub-views
Refactor <ErrorMessage /> component to use new error utils
Refactor error handling to throw errors, to be caught by error boundaries
Add a container component for the header, for easier reuse (necessary in full view error)
Add logic to make links in the header also work as plain anchor links (necessary in full view error)
Add a generic not found view for routes not matching any react router route
Small related styling tweaks
Add top-most error boundary to oauth app
Add error view for oauth app
Add detection of PAGE_DATA errors in both console and oauth

Notes for Reviewers

Using this approach, we can throw errors from anywhere in the render tree and have them handled by the nearest error boundary. I believe the exact granularity of where to place error boundaries will be figured out by us as we go. Also, the exact content of the error views needs to be adapted by us later based on user feedback and progressive insight. The overall goal should be to hint the user towards problem resolution whenever possible.

Otherwise:

I decided against using a shared error view for oauth and console, I sense that we will need to distinguish between them more clearly later. For now they use more or less the same (duplicated) error view component
The error boundary and page data error check could be made a part of the shared <Init />-component but I decided to do this more explicitly for now

bafonins · 2019-05-10T15:48:32Z

pkg/webui/console/views/application/index.js

    if (error) {
-      return 'ERROR'
+      return <FullViewError error={error} />


According to this comment #659 (comment) we want to avoid this, right?
namely:

We would then have to use portals to be able to show full page errors. Otherwise, we could only display within the boundaries of the dom that the component is rendered into. E.g. in case of 404 of an entity we would still have to show the sidebar, which is not what we want. And even with portals, it would still render unnecessarily, unless we add logic to prevent that.

here is how the page looks when using this approach:

bafonins · 2019-05-10T15:49:46Z

pkg/webui/lib/errors/utils.js

+)
+
+/**
+ * Returns wether the error has a shape that is well-known


misleading comment imo
according to the comment i would expect the function to return true when the error shape is well-known

bafonins · 2019-05-10T15:51:25Z

pkg/webui/lib/errors/status-code-messages.js

+  '407': 'Proxy Authentication Required',
+  '408': 'Request Timeout',
+  '409': 'Conflict',
+  '410': 'Gone',


Do we really want to have this?
I dont see how the user can benefit from seeing Gone in the error view.

I have removed all status codes here that do not have a connected grpc error.

coveralls · 2019-05-14T09:28:20Z

Coverage decreased (-0.01%) to 73.464% when pulling 852aa4e on feature/695-frontend-error-handling into 56f8af3 on master.

kschiffer · 2019-05-14T09:34:49Z

Description updated

kschiffer · 2019-05-14T09:37:37Z

pkg/webui/console/views/error/full-view.styl

@@ -12,21 +12,31 @@
 // See the License for the specific language governing permissions and


This is wrongfully labeled as moved file by github 🤷‍♂.

bafonins · 2019-05-14T09:50:46Z

pkg/webui/lib/components/error-view.js

+}
+
+ErrorView.propTypes = {
+  ErrorComponent: PropTypes.element.isRequired,


causes:

Warning: Failed prop type: Invalid prop `ErrorComponent` of type `function` supplied to `ErrorView`, expected a single ReactElement. in ErrorView (created by ConsoleApp) in ConsoleApp (created by WithEnv(ConsoleApp)) in WithEnv(ConsoleApp) (created by Route) in Route (created by withRouter(WithEnv(ConsoleApp))) in withRouter(WithEnv(ConsoleApp)) (created by Console) in SideNavigationProvider (created by Console) in BreadcrumbsProvider (created by Console) in Router (created by ConnectedRouter) in ConnectedRouter (created by Connect(ConnectedRouter)) in Connect(ConnectedRouter) (created by Console) in IntlProvider (created by UserLocale) in UserLocale (created by WithEnv(UserLocale)) in WithEnv(UserLocale) (created by Connect(WithEnv(UserLocale))) in Connect(WithEnv(UserLocale)) (created by Console) in Init (created by Connect(Init)) in Connect(Init) (created by Console) in Provider (created by Console) in EnvProvider (created by Console) in Console

bafonins · 2019-05-14T09:54:54Z

pkg/webui/components/header/index.js

@@ -129,6 +138,8 @@ Header.propTypes = {
    action: PropTypes.func,
    icon: PropTypes.string,
  })),
+  /** Flag identifying whether links should be rendered as plain anchor link */
+  anchored: PropTypes.bool,


apparently, this breaks the header:

bafonins · 2019-05-14T09:56:23Z

pkg/webui/console/containers/header/index.js

+  user: state.user.user,
+}))
+@bind
+export default class Header extends Component {


proptypes missing

bafonins · 2019-05-14T10:00:17Z

pkg/webui/console/containers/gateway-statistics/index.js

@@ -24,7 +24,7 @@ import Icon from '../../../components/icon'
 import DateTime from '../../../lib/components/date-time'
 import Message from '../../../lib/components/message'
 import Button from '../../../components/button'
-import { isNotFoundError, isErrorTranslated } from '../../../lib/errors'
+import { isNotFoundError, isTranslated } from '../../../lib/errors/utils'


isNotFoundError throws TypeError when creating a new gateway

bafonins · 2019-05-14T10:02:36Z

pkg/webui/console/views/application-collaborators/index.js

-        <Route path={`${match.path}/add`} component={ApplicationCollaboratorAdd} />
-        <Route path={`${match.path}/:collaboratorId`} component={ApplicationCollaboratorEdit} />
-      </Switch>
+      <ErrorView ErrorComponent={SubViewError}>


ErrorView is not imported to the file. causes ReferenceError

kschiffer · 2019-05-14T16:22:12Z

Updated once more

kschiffer added the c/console This is related to the Console label May 9, 2019

kschiffer added this to the May 2019 milestone May 9, 2019

kschiffer requested a review from bafonins May 9, 2019 15:34

kschiffer self-assigned this May 9, 2019

bafonins reviewed May 10, 2019

View reviewed changes

kschiffer force-pushed the feature/695-frontend-error-handling branch 2 times, most recently from fb631e4 to a91ba36 Compare May 14, 2019 09:15

kschiffer marked this pull request as ready for review May 14, 2019 09:17

kschiffer requested a review from bafonins May 14, 2019 09:18

kschiffer commented May 14, 2019

View reviewed changes

bafonins reviewed May 14, 2019

View reviewed changes

bafonins suggested changes May 14, 2019

View reviewed changes

kschiffer requested a review from bafonins May 14, 2019 13:00

kschiffer force-pushed the feature/695-frontend-error-handling branch 2 times, most recently from 1581eb0 to 79be69b Compare May 14, 2019 16:15

bafonins approved these changes May 15, 2019

View reviewed changes

kschiffer added 6 commits May 15, 2019 11:31

console: Extend error library

1cd775a

console: Update error lib references

04582bb

console: Add anchor link option to navbar

feed3cc

console: Add anchor link option to profile dropdown

7cd9788

console: Add anchor link option to header

81a59fb

console: Add header container component

1ae39cd

kschiffer force-pushed the feature/695-frontend-error-handling branch 2 times, most recently from eeb4dd6 to d9a114e Compare May 15, 2019 10:10

console: Add console error view

16e2153

kschiffer added 21 commits May 15, 2019 12:12

console,oauth: Update error message component

a899300

console,oauth: Add error boundary component

2760ef1

console: Add error boundary to console app

547a773

console: Handle error in application view

e7ba963

console: Refactor header styles

e57ad7b

console: Extend error view styles

5317d1d

console: Add sub-view error component

7faf442

console: Add error boundary to api key views

67b7cc6

console: Throw errors in application api key views

722bdda

console: Add error boundary to collaborator views

c6f9f62

console: Throw errors in collaborator views

48f83a8

console: Throw errors in entity views

8001ac1

console: Add error boundary to gateway api key views

e2bcc7b

console: Move router context to header container

441059c

console: Throw errors in gateway api key views

cf8e062

console: Add not found route

90727f5

oauth: Add full view error

a3d1eb5

oauth: Add topmost error boundary to oauth app

ed082ae

console: Add handling of page data errors

7b01c9f

console: Fix api key selectors

3ba8f64

dev: Update locales

7c9584b

kschiffer force-pushed the feature/695-frontend-error-handling branch from d9a114e to 7c9584b Compare May 15, 2019 10:13

oauth: Add not found route

852aa4e

kschiffer merged commit 862b340 into master May 15, 2019

kschiffer deleted the feature/695-frontend-error-handling branch May 15, 2019 11:38

This was referenced May 15, 2019

Come up with a proper way to display errors in frontend #41

Closed

Use error library to streamline error handling in frontend #659

Closed

Console MVP #629

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Frontend Error Handling #667

Frontend Error Handling #667

kschiffer commented May 9, 2019 •

edited

Loading

bafonins May 10, 2019

bafonins May 10, 2019

bafonins May 10, 2019

kschiffer May 14, 2019

coveralls commented May 14, 2019 •

edited

Loading

kschiffer commented May 14, 2019

kschiffer May 14, 2019

bafonins May 14, 2019

bafonins May 14, 2019

bafonins May 14, 2019

bafonins May 14, 2019

bafonins May 14, 2019

kschiffer commented May 14, 2019

		@@ -12,21 +12,31 @@
		// See the License for the specific language governing permissions and

Frontend Error Handling #667

Frontend Error Handling #667

Conversation

kschiffer commented May 9, 2019 • edited Loading

Summary

Changes

Notes for Reviewers

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coveralls commented May 14, 2019 • edited Loading

kschiffer commented May 14, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kschiffer commented May 14, 2019

kschiffer commented May 9, 2019 •

edited

Loading

coveralls commented May 14, 2019 •

edited

Loading