Merge pull request #884 from openzim/better_error_message

openzim · May 2, 2024 · 59e9a39 · 59e9a39
2 parents 0cdf6f6 + c5c3ad4
commit 59e9a39
Show file tree

Hide file tree

Showing 3 changed files with 45 additions and 8 deletions.
diff --git a/src/file_compound.cpp b/src/file_compound.cpp
@@ -41,10 +41,13 @@ void FileCompound::addPart(FilePart* fpart)
   _fsize += fpart->size();
 }
 
-std::shared_ptr<FileCompound> FileCompound::openSinglePieceOrSplitZimFile(std::string filename) {
+std::shared_ptr<FileCompound> FileCompound::openSinglePieceOrSplitZimFile(const std::string& original_filename) {
   std::shared_ptr<FileCompound> fileCompound;
+  bool multi_parts_asked = false;
+  auto filename = original_filename;
   if (filename.size() > 6 && filename.substr(filename.size()-6) == ".zimaa") {
     filename.resize(filename.size()-2);
+    multi_parts_asked = true;
   } else {
   try {
       fileCompound = std::make_shared<FileCompound>(filename);
@@ -54,6 +57,13 @@ std::shared_ptr<FileCompound> FileCompound::openSinglePieceOrSplitZimFile(std::s
   if ( !fileCompound ) {
     fileCompound = std::make_shared<FileCompound>(filename, FileCompound::MultiPartToken::Multi);
   }
+
+  if (fileCompound->empty()) {
+    // We haven't found any part
+    throw std::runtime_error(Formatter() << "Error opening "
+                             << (multi_parts_asked ? "as a split " : "")
+                             << "ZIM file: " << original_filename);
+  }
   return fileCompound;
 }
 
@@ -80,10 +90,6 @@ FileCompound::FileCompound(const std::string& base_filename, MultiPartToken _tok
   } catch (std::runtime_error& e) {
     // This catch acts as a break for the double loop.
   }
-  if (empty()) {
-    // We haven't found any part
-    throw std::runtime_error(Formatter() << "Error opening as a split file: " << base_filename);
-  }
 }
 
 #ifndef _WIN32

diff --git a/src/file_compound.h b/src/file_compound.h
@@ -57,7 +57,7 @@ class FileCompound : private std::map<Range, FilePart*, less_range> {
     enum class MultiPartToken { Multi };
 
   public: // functions
-    static std::shared_ptr<FileCompound> openSinglePieceOrSplitZimFile(std::string filename);
+    static std::shared_ptr<FileCompound> openSinglePieceOrSplitZimFile(const std::string& filename);
     explicit FileCompound(const std::string& filename);
     explicit FileCompound(const std::string& filename, MultiPartToken token);
 

diff --git a/test/archive.cpp b/test/archive.cpp
@@ -284,8 +284,39 @@ TEST(ZimArchive, openDontFallbackOnNonSplitZimArchive)
 
   for (auto& testfile: getDataFilePath(fname)) {
     const TestContext ctx{ {"path", testfile.path+"aa" } };
-    std::unique_ptr<zim::Archive> archive;
-    EXPECT_THROW( archive.reset(new zim::Archive(testfile.path+"aa")), std::runtime_error) << ctx;
+    try {
+      zim::Archive(testfile.path+"aa");
+      FAIL(); // Exception is expected
+    } catch(const std::runtime_error& e) {
+      const std::string expected = std::string("Error opening as a split ZIM file: ") + testfile.path + "aa";
+      EXPECT_EQ(expected, e.what()) << ctx;
+    }
+  }
+}
+
+TEST(ZimArchive, openNonExistantZimArchive)
+{
+  const std::string fname = "non_existant.zim";
+
+  try {
+    zim::Archive archive(fname);
+    FAIL(); // Exception is expected
+  } catch(const std::runtime_error& e) {
+    const std::string expected = std::string("Error opening ZIM file: ") + fname;
+    EXPECT_EQ(expected, e.what()) << fname;
+  }
+}
+
+TEST(ZimArchive, openNonExistantZimSplitArchive)
+{
+  const std::string fname = "non_existant.zimaa";
+
+  try {
+    zim::Archive archive(fname);
+    FAIL(); // Exception is expected
+  } catch(const std::runtime_error& e) {
+    const std::string expected = std::string("Error opening as a split ZIM file: ") + fname;
+    EXPECT_EQ(expected, e.what()) << fname;
   }
 }