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 undefined access on not-existing segmentation layer #5583

Merged
merged 4 commits into from
Jun 24, 2021

Conversation

philippotto
Copy link
Member

@philippotto philippotto commented Jun 22, 2021

URL of deployed dev instance (used for testing):

Steps to test:

  • open a dataset without segmentation layer in view mode and switch to the Meshes tab (within a hybrid annotation, an segmentation layer would always exist)
  • ensure that the console doesn't print Uncaught (in promise) TypeError: UnhandledRejection: "TypeError: Cannot read property 'fallbackLayer' of null"

Issues:

  • fixes airbrake reports I saw with Uncaught (in promise) TypeError: UnhandledRejection: "TypeError: Cannot read property 'fallbackLayer' of null"

@philippotto philippotto self-assigned this Jun 22, 2021
@@ -59,7 +59,7 @@ export const stlIsosurfaceConstants = {

// This file defines the component MeshesView.

const mapStateToProps = (state: OxalisState) => ({
const mapStateToProps = (state: OxalisState): * => ({
Copy link
Member Author

@philippotto philippotto Jun 22, 2021

Choose a reason for hiding this comment

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

Took me a while to figure out why the errors regarding the Props were not caught by flow. It turns out, flow simply assumed the props to be any. The reason for this is that the extraction of the return type for these functions here didn't work. After a bit of digging I found out that the * as a return type helps 🤦

Here's the comment: facebook/flow#7071 (comment)
The issue was opened 2.5 years ago. Yet another reason to ditch flow?

I also looked for ExtractReturn and there is only one other usage which doesn't seem to suffer from the same bug. So, hopefully no other bad suprises.

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.

Nice find and fix! Thank you for spending the extra time to get to the root of why flow didn't catch this 🎉

CHANGELOG.unreleased.md Outdated Show resolved Hide resolved
@philippotto philippotto enabled auto-merge (squash) June 24, 2021 07:11
@philippotto philippotto merged commit 44080a7 into master Jun 24, 2021
@philippotto philippotto deleted the fix-undefined-mesh branch June 14, 2022 11:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants