Fix exception thrown given invalid arguments with compression enabled (ros2#488)

* Remove exception throw in dtor
Update logging errors to avoid duplication

Signed-off-by: Devin Bonnie <dbbonnie@amazon.com>

* Cleanup reset logic
Add reset documentation

Signed-off-by: Devin Bonnie <dbbonnie@amazon.com>

* Fix cpplint error

Signed-off-by: Devin Bonnie <dbbonnie@amazon.com>

* Fix docstring

Signed-off-by: Devin Bonnie <dbbonnie@amazon.com>

Author: dabonnie

rosbag2_compression/include/rosbag2_compression/sequential_compression_writer.hpp

@@ -75,6 +75,10 @@ class ROSBAG2_COMPRESSION_PUBLIC SequentialCompressionWriter
     const rosbag2_cpp::StorageOptions & storage_options,
     const rosbag2_cpp::ConverterOptions & converter_options) override;
 
+  /**
+   * Attempt to compress the last open file and reset the storage and storage factory.
+   * This method must be exception safe because it is called by the destructor.
+   */
   void reset() override;
 
   /**

rosbag2_compression/src/rosbag2_compression/sequential_compression_writer.cpp

@@ -133,8 +133,8 @@ void SequentialCompressionWriter::open(
     throw std::runtime_error{"No storage could be initialized. Abort"};
   }
 
-  if (storage_options.max_bagfile_size != 0 &&
-    storage_options.max_bagfile_size < storage_->get_minimum_split_file_size())
+  if (max_bagfile_size_ != 0 &&
+    max_bagfile_size_ < storage_->get_minimum_split_file_size())
   {
     std::stringstream error;
     error << "Invalid bag splitting size given. Please provide a value greater than " <<
@@ -149,11 +149,7 @@ void SequentialCompressionWriter::open(
 
 void SequentialCompressionWriter::reset()
 {
-  if (!base_folder_.empty()) {
-    if (!compressor_) {
-      throw std::runtime_error{"Compressor was not opened!"};
-    }
-
+  if (!base_folder_.empty() && compressor_) {
     // Reset may be called before initializing the compressor (ex. bad options).
     // We compress the last file only if it hasn't been compressed earlier (ex. in split_bagfile()).
     if (compression_options_.compression_mode == rosbag2_compression::CompressionMode::FILE &&
@@ -223,7 +219,7 @@ void SequentialCompressionWriter::remove_topic(
 void SequentialCompressionWriter::compress_last_file()
 {
   if (!compressor_) {
-    throw std::runtime_error{"Compressor was not opened!"};
+    throw std::runtime_error{"compress_last_file: Compressor was not opened!"};
   }
 
   const auto to_compress = rcpputils::fs::path{metadata_.relative_file_paths.back()};

rosbag2_compression/test/rosbag2_compression/test_sequential_compression_writer.cpp

@@ -89,6 +89,7 @@ TEST_F(SequentialCompressionWriterTest, open_throws_on_invalid_splitting_size)
   ON_CALL(*storage_, get_minimum_split_file_size()).WillByDefault(Return(min_split_file_size));
   auto storage_options = rosbag2_cpp::StorageOptions{};
   storage_options.max_bagfile_size = max_bagfile_size;
+  storage_options.uri = "foo.bar";
 
   auto sequential_writer = std::make_unique<rosbag2_compression::SequentialCompressionWriter>(
     compression_options,