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

Make sure an incorrect phylogeny doesn't break the rest of the UI #187

Closed
gaurav opened this issue Jan 28, 2020 · 5 comments · Fixed by #308
Closed

Make sure an incorrect phylogeny doesn't break the rest of the UI #187

gaurav opened this issue Jan 28, 2020 · 5 comments · Fixed by #308
Assignees
Labels
bug difficulty: easy Should be easy to do (1-2 days) programming only No discussion necessary; just pure programming needed
Milestone

Comments

@gaurav
Copy link
Member

gaurav commented Jan 28, 2020

Anna appears to have found a case where a malformed Newick string causes the phyloreference summary page to blank out because of an internal error. We should trap this and make sure the page can be rendered even with a malformed Newick string.

@gaurav gaurav added this to the Klados v1.0 milestone Nov 10, 2021
@gaurav
Copy link
Member Author

gaurav commented Dec 15, 2021

For example, (Parasuchia,(rauisuchians,Aetosauria,(sphenosuchians,(protosuchians,(mesosuchians,(Hylaeochampsrostres)Crocodylia))Eusuchia)Mesoeucrocodylia)Crocodyliformes)Crocodylomorpha))root; seems to cause this problem.

@gaurav gaurav added difficulty: easy Should be easy to do (1-2 days) programming only No discussion necessary; just pure programming needed bug labels Dec 15, 2021
gaurav added a commit that referenced this issue Jan 25, 2022
I'm not sure if it'll work for other cases, but it fixes that particular
case.
@gaurav
Copy link
Member Author

gaurav commented Mar 29, 2022

The root problem is that PhylogenyWrapper can throw an exception (phyloref/phyx.js#116), which PhyxWrapper doesn't catch, and so it propagates up to us. The right solution is probably to modify the UI in order to display a banner at the top of the screen saying, "Phylogeny X is invalid and will be ignored.", and then implement some logic so that any invalid phylogenies are consistently ignored whenever PhyxWrapper is used, i.e. when exporting the Phyx file as JSON-LD or n-Triples in OWL.

@gaurav
Copy link
Member Author

gaurav commented Aug 24, 2022

Might not need to be in milestone; please review.

@gaurav
Copy link
Member Author

gaurav commented May 24, 2023

I am unable to replicate this now, so it looks like we've successfully caught the exceptions being generated. Closing.

(Note that we still want to improve one of the error messages in #196, but that doesn't break the entire phylogeny.)

@gaurav
Copy link
Member Author

gaurav commented Feb 14, 2024

This occurs on the master branch again, but PR #308 fixes it.

@gaurav gaurav reopened this Feb 14, 2024
@gaurav gaurav self-assigned this Feb 14, 2024
gaurav added a commit that referenced this issue Mar 17, 2024
Phylogenies and phyloreferences can throw exceptions when you attempt to load them or convert them into JSON-LD/n-Quads, which we've previously ignored. This PR adds `try {} catch {}` blocks around such invocations so that we can produce an error instead of having the entire Vue app crash. It also adds the download filename to the Advanced Modal dialog download button for completeness.

Also includes a minor fix to the formatting of a few lines and checks for unbalanced single or double quotes in the Newick file.

Closes #187.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug difficulty: easy Should be easy to do (1-2 days) programming only No discussion necessary; just pure programming needed
Projects
None yet
1 participant