feat(silo): improve error handling - error if duplicate nodeIds in newick or auspiceJson

Author: anna-parker

src/silo/common/phylo_tree.cpp

@@ -35,6 +35,11 @@ std::shared_ptr<TreeNode> parse_auspice_tree(
    }
 
    // Insert node into the map *after* children so it's fully constructed
+   if (node_map.find(node->node_id) != node_map.end()) {
+      throw silo::preprocessing::PreprocessingException(
+         fmt::format("Duplicate node ID found in Newick string: '{}'", node->node_id.string)
+      );
+   }
    node_map[node->node_id] = node;
    return node;
 }
@@ -150,6 +155,11 @@ std::shared_ptr<TreeNode> parseSubtree(
    node->node_id = parseLabel(sv);
    skipWhitespace(sv);
 
+   if (node_map.find(node->node_id) != node_map.end()) {
+      throw silo::preprocessing::PreprocessingException(
+         fmt::format("Duplicate node ID found in Newick string: '{}'", node->node_id.string)
+      );
+   }
    node_map[node->node_id] = node;
 
    return node;
@@ -179,7 +189,7 @@ PhyloTree PhyloTree::fromNewickString(const std::string& newick_string) {
       }
    } catch (const std::exception& e) {
       throw silo::preprocessing::PreprocessingException(
-         fmt::format("Error when parsing the Newick string: '{}'", newick_string)
+         fmt::format("Error when parsing the Newick string '{}': {}", newick_string, e.what())
       );
    }
 
@@ -207,7 +217,7 @@ PhyloTree PhyloTree::fromNewickFile(const std::filesystem::path& newick_path) {
       return fromNewickString(contents.str());
    } catch (const std::exception& e) {
       throw silo::preprocessing::PreprocessingException(
-         fmt::format("Error when parsing the Newick file: '{}'", newick_path.string())
+         fmt::format("Error when parsing the Newick file '{}': {}", newick_path.string(), e.what())
       );
    }
 }

src/silo/common/phylo_tree.test.cpp

@@ -50,6 +50,29 @@ TEST(PhyloTree, throwsOnInvalidJSON) {
    );
 }
 
+TEST(PhyloTree, throwsOnInvalidAuspiceJSONDuplicateNodeId) {
+   EXPECT_THROW(
+      PhyloTree::fromAuspiceJSONString(R"({  
+  "version": "schema version",  
+  "meta": {},  
+  "tree": {  
+    "name": "ROOT",  
+    "children": [  
+      {  
+        "name": "CHILD",  
+        "children": [  
+          {  
+            "name": "CHILD"  
+          }  
+        ]  
+      }  
+    ]  
+  }  
+})"),
+      silo::preprocessing::PreprocessingException
+   );
+}
+
 TEST(PhyloTree, correctlyParsesFromNewick) {
    auto phylo_tree_file = PhyloTree::fromNewickString("((CHILD2)CHILD)ROOT;");
    ASSERT_EQ(phylo_tree_file.nodes.size(), 3);
@@ -95,3 +118,9 @@ TEST(PhyloTree, throwsOnInvalidNewickNoSemicolon) {
       silo::preprocessing::PreprocessingException
    );
 }
+
+TEST(PhyloTree, throwsOnInvalidNewickWithDuplicateNodeId) {
+   EXPECT_THROW(
+      PhyloTree::fromNewickString("((CHILD)CHILD)ROOT"), silo::preprocessing::PreprocessingException
+   );
+}

testBaseData/unitTestDummyDataset/phylogenetic_tree.nwk

@@ -1 +1 @@
-((key1, key2), ((key3, key4), key5));
+((key1, key2)inner1, ((key3, key4)inner2, key5)inner3)root;