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

Avoid more null-errors in isosurface and mesh handling #5142

Merged
merged 5 commits into from
Feb 11, 2021

Conversation

philippotto
Copy link
Member

@philippotto philippotto commented Feb 5, 2021

The meshes UI could provoke errors when viewing a dataset without segmentation layer or when update-permissions are missing (for example, in view mode).

URL of deployed dev instance (used for testing):

  • https://___.webknossos.xyz

Steps to test:

  • I tested the cross-product of the following factors:
    • dataset with and without segmentation layer (I used ROI2017_wkw and data_types)
    • open dataset in view mode, with a hybrid and with a skeleton-only annotation
  • In each constellation:
    • make sure the "load centered isosurface" behaves correctly (it should only be disabled if there is no segmentation layer). if the centered cell is 0, the button is available, but should emit a warning toast.
    • the import buttons are only enabled if allowUpdate is true
    • importing an meshes/isosurface STLs should "work as expected". see here for sample STLs.

Issues:


@philippotto philippotto self-assigned this Feb 5, 2021
Copy link
Member

@daniel-wer daniel-wer left a comment

Choose a reason for hiding this comment

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

LGTM 👍

In #4935 you wrote:

Related: this error only appears in a very obscure report in airbrake: https://scm.airbrake.io/projects/95438/groups/2835680766133594723?tab=notice-detail

Instead of [object Object], there should be a proper error message. I suggest to investigate this, too, before fixing the actual problem, since it's crucial to have robust error reporting.

Were you able to have a look at that as well? :)

@philippotto philippotto changed the title Avoid more checks in isosurface and mesh handling Avoid more null-errors in isosurface and mesh handling Feb 8, 2021
@philippotto
Copy link
Member Author

Related: this error only appears in a very obscure report in airbrake: https://scm.airbrake.io/projects/95438/groups/2835680766133594723?tab=notice-detail
Instead of [object Object], there should be a proper error message. I suggest to investigate this, too, before fixing the actual problem, since it's crucial to have robust error reporting.

Were you able to have a look at that as well? :)

Good point! I investigated this and if the sagas crash, the try-catch-block potentially receives the rejection reason of a promise, which doesn't need to be an error instance. for this case, the default stringification would produce [object object]. I fixed this by applying json.stringify when necessary. It's a bit of a bummer that rejection reasons and errors seem to be mixed in the catch-clause, but I cannot think of a more elegant solution right now.

@bulldozer-boy bulldozer-boy bot merged commit 11fd251 into master Feb 11, 2021
@bulldozer-boy bulldozer-boy bot deleted the isosurface-fixes branch February 11, 2021 09:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants