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

Fix crash in publication page and add error boundaries #6700

Merged
merged 11 commits into from
Dec 19, 2022

Conversation

philippotto
Copy link
Member

@philippotto philippotto commented Dec 15, 2022

  • adds error boundaries to the individual routes (except for the tracing views since these have their own boundaries)
    • a "nice" error page will be shown to the user and the error will be logged to airbrake, too
  • adds top-level error boundary for route-independent react errors
  • fixes rendering error when a publication doesn't have any active datasets or annotations

URL of deployed dev instance (used for testing):

  • https://___.webknossos.xyz

Steps to test:

  • the publication fix is a bit tedious to set up. I don't think, it needs additional testing. It will look like this (a bit naked, but this should be only an edge case, anyway):
    image
  • the error boundaries can be tested by:
    • navigating to admin > users
    • image
    • clicking "help" in the navbar and "ask a question" (this error boundary is route independent)
      image

Issues:


(Please delete unneeded items, merge only when none are left open)

@philippotto philippotto self-assigned this Dec 15, 2022
Copy link
Member Author

@philippotto philippotto left a comment

Choose a reason for hiding this comment

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

review will be easier by ignoring whitespace changes

frontend/javascripts/admin/user/user_list_view.tsx Outdated Show resolved Hide resolved
frontend/javascripts/navbar.tsx Outdated Show resolved Hide resolved
Copy link
Member

@normanrz normanrz left a comment

Choose a reason for hiding this comment

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

code looks good. i'll do some testing now

frontend/javascripts/admin/user/user_list_view.tsx Outdated Show resolved Hide resolved
@normanrz
Copy link
Member

Tested -> works. I was wondering if the toast is still required? Seems a bit redundant.

@philippotto
Copy link
Member Author

Tested -> works. I was wondering if the toast is still required? Seems a bit redundant.

I assume you tested it locally, right? Orthogonal to the error boundaries, there is a global window.onerror handler (mainly for errors unrelated to react). Due to a peculiarity of react, that event handler is triggered again after an error boundary was activated, but only in DEV mode. See this react issue. So, in the production build, there won't be the redundant error toast. I just tested it at https://fixpublicationpage.webknossos.xyz/users.

@philippotto philippotto enabled auto-merge (squash) December 19, 2022 08:12
@philippotto philippotto merged commit d2314b7 into master Dec 19, 2022
@philippotto philippotto deleted the fix-publication-page branch December 19, 2022 08:42
hotzenklotz added a commit that referenced this pull request Dec 20, 2022
…cing

* 'master' of github.com:scalableminds/webknossos:
  Automatically open (and close) quick select settings when labeling in… (#6706)
  Add Terms of Service Acceptance Concept (#6632)
  Fix crash in publication page and add error boundaries (#6700)
  temporarily disable vx related polling (#6702)
  add protected and private modifiers to DAO hierarchy (#6698)
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.

2 participants