Fix logging bugs and improve error handling

- Convert parse functions to return Result instead of panicking
- Fix `LOGGING_PYO3` lifecycle by resetting flag when switching modes
- Seed `FileWriter` rotation counter from existing file size
- Add tests for logging error cases and string stripping
- Update docs to correct component_levels type from LogLevel to str

Author: cjdsellers

crates/common/src/ffi/logging.rs

@@ -30,8 +30,9 @@ use nautilus_model::identifiers::TraderId;
 use crate::{
     enums::{LogColor, LogLevel},
     logging::{
-        headers,
+        headers, init_logging,
         logger::{self, LogGuard, LoggerConfig},
+        map_log_level_to_filter, parse_component_levels,
         writer::FileWriterConfig,
     },
 };
@@ -101,11 +102,12 @@ pub unsafe extern "C" fn logging_init(
     max_file_size: u64,
     max_backup_count: u32,
 ) -> LogGuard_API {
-    let level_stdout = crate::logging::map_log_level_to_filter(level_stdout);
-    let level_file = crate::logging::map_log_level_to_filter(level_file);
+    let level_stdout = map_log_level_to_filter(level_stdout);
+    let level_file = map_log_level_to_filter(level_file);
 
     let component_levels_json = unsafe { optional_bytes_to_json(component_levels_ptr) };
-    let component_levels = crate::logging::parse_component_levels(component_levels_json);
+    let component_levels = parse_component_levels(component_levels_json)
+        .expect("Failed to parse component log levels");
 
     let config = LoggerConfig::new(
         level_stdout,
@@ -130,11 +132,12 @@ pub unsafe extern "C" fn logging_init(
     let file_config = FileWriterConfig::new(directory, file_name, file_format, file_rotate);
 
     if u8_as_bool(is_bypassed) {
-        crate::logging::logging_set_bypass();
+        logging_set_bypass();
     }
 
     LogGuard_API(Box::new(
-        crate::logging::init_logging(trader_id, instance_id, config, file_config).unwrap(),
+        init_logging(trader_id, instance_id, config, file_config)
+            .expect("Failed to initialize logging"),
     ))
 }
 

crates/common/src/logging/mod.rs

@@ -189,43 +189,42 @@ pub const fn map_log_level_to_filter(log_level: LogLevel) -> LevelFilter {
 
 /// Parses a string into a [`LevelFilter`].
 ///
-/// # Panics
+/// # Errors
 ///
-/// Panics if the provided string is not a valid `LevelFilter`.
-#[must_use]
-pub fn parse_level_filter_str(s: &str) -> LevelFilter {
+/// Returns an error if the provided string is not a valid `LevelFilter`.
+pub fn parse_level_filter_str(s: &str) -> anyhow::Result<LevelFilter> {
     let mut log_level_str = s.to_string().to_uppercase();
     if log_level_str == "WARNING" {
         log_level_str = "WARN".to_string();
     }
     LevelFilter::from_str(&log_level_str)
-        .unwrap_or_else(|_| panic!("Invalid `LevelFilter` string, was {log_level_str}"))
+        .map_err(|_| anyhow::anyhow!("Invalid log level string: '{s}'"))
 }
 
-#[must_use]
 /// Parses component-specific log levels from a JSON value map.
 ///
-/// # Panics
+/// # Errors
 ///
-/// Panics if a JSON value in the map is not a string representing a log level.
+/// Returns an error if a JSON value in the map is not a string or is not a valid log level.
 pub fn parse_component_levels(
     original_map: Option<HashMap<String, serde_json::Value>>,
-) -> HashMap<Ustr, LevelFilter> {
+) -> anyhow::Result<HashMap<Ustr, LevelFilter>> {
     match original_map {
         Some(map) => {
             let mut new_map = HashMap::new();
             for (key, value) in map {
                 let ustr_key = Ustr::from(&key);
-                // Expect the JSON value to be a string representing a log level
-                let s = value
-                    .as_str()
-                    .expect("Invalid component log level: expected string");
-                let lvl = parse_level_filter_str(s);
+                let s = value.as_str().ok_or_else(|| {
+                    anyhow::anyhow!(
+                        "Component log level for '{key}' must be a string, got: {value}"
+                    )
+                })?;
+                let lvl = parse_level_filter_str(s)?;
                 new_map.insert(ustr_key, lvl);
             }
-            new_map
+            Ok(new_map)
         }
-        None => HashMap::new(),
+        None => Ok(HashMap::new()),
     }
 }
 
@@ -253,3 +252,111 @@ pub fn log_task_aborted(task_name: &str) {
 pub fn log_task_error(task_name: &str, e: &anyhow::Error) {
     tracing::error!("Error in task '{task_name}': {e}");
 }
+
+#[cfg(test)]
+mod tests {
+    use rstest::rstest;
+
+    use super::*;
+
+    #[rstest]
+    #[case("DEBUG", LevelFilter::Debug)]
+    #[case("debug", LevelFilter::Debug)]
+    #[case("Debug", LevelFilter::Debug)]
+    #[case("DeBuG", LevelFilter::Debug)]
+    #[case("INFO", LevelFilter::Info)]
+    #[case("info", LevelFilter::Info)]
+    #[case("WARNING", LevelFilter::Warn)]
+    #[case("warning", LevelFilter::Warn)]
+    #[case("WARN", LevelFilter::Warn)]
+    #[case("warn", LevelFilter::Warn)]
+    #[case("ERROR", LevelFilter::Error)]
+    #[case("error", LevelFilter::Error)]
+    #[case("OFF", LevelFilter::Off)]
+    #[case("off", LevelFilter::Off)]
+    #[case("TRACE", LevelFilter::Trace)]
+    #[case("trace", LevelFilter::Trace)]
+    fn test_parse_level_filter_str_case_insensitive(
+        #[case] input: &str,
+        #[case] expected: LevelFilter,
+    ) {
+        let result = parse_level_filter_str(input).unwrap();
+        assert_eq!(result, expected);
+    }
+
+    #[rstest]
+    #[case("INVALID")]
+    #[case("DEBG")]
+    #[case("WARNINGG")]
+    #[case("")]
+    #[case("INFO123")]
+    fn test_parse_level_filter_str_invalid_returns_error(#[case] invalid_input: &str) {
+        let result = parse_level_filter_str(invalid_input);
+
+        assert!(result.is_err());
+        assert!(
+            result
+                .unwrap_err()
+                .to_string()
+                .contains("Invalid log level")
+        );
+    }
+
+    #[rstest]
+    fn test_parse_component_levels_valid() {
+        let mut map = HashMap::new();
+        map.insert(
+            "Strategy1".to_string(),
+            serde_json::Value::String("DEBUG".to_string()),
+        );
+        map.insert(
+            "Strategy2".to_string(),
+            serde_json::Value::String("info".to_string()),
+        );
+
+        let result = parse_component_levels(Some(map)).unwrap();
+
+        assert_eq!(result.len(), 2);
+        assert_eq!(result[&Ustr::from("Strategy1")], LevelFilter::Debug);
+        assert_eq!(result[&Ustr::from("Strategy2")], LevelFilter::Info);
+    }
+
+    #[rstest]
+    fn test_parse_component_levels_non_string_value_returns_error() {
+        let mut map = HashMap::new();
+        map.insert(
+            "Strategy1".to_string(),
+            serde_json::Value::Number(123.into()),
+        );
+
+        let result = parse_component_levels(Some(map));
+
+        assert!(result.is_err());
+        assert!(result.unwrap_err().to_string().contains("must be a string"));
+    }
+
+    #[rstest]
+    fn test_parse_component_levels_invalid_level_returns_error() {
+        let mut map = HashMap::new();
+        map.insert(
+            "Strategy1".to_string(),
+            serde_json::Value::String("INVALID_LEVEL".to_string()),
+        );
+
+        let result = parse_component_levels(Some(map));
+
+        assert!(result.is_err());
+        assert!(
+            result
+                .unwrap_err()
+                .to_string()
+                .contains("Invalid log level")
+        );
+    }
+
+    #[rstest]
+    fn test_parse_component_levels_none_returns_empty() {
+        let result = parse_component_levels(None).unwrap();
+        assert_eq!(result.len(), 0);
+    }
+}

crates/common/src/logging/writer.rs

@@ -226,16 +226,26 @@ impl FileWriter {
             .append(true)
             .open(file_path.clone())
         {
-            Ok(file) => Some(Self {
-                json_format,
-                buf: BufWriter::new(file),
-                path: file_path,
-                file_config,
-                trader_id,
-                instance_id,
-                level: fileout_level,
-                cur_file_date: Utc::now().date_naive(),
-            }),
+            Ok(file) => {
+                // Seed cur_file_size from existing file length if rotation is enabled
+                let mut file_config = file_config;
+                if let Some(ref mut rotate_config) = file_config.file_rotate
+                    && let Ok(metadata) = file.metadata()
+                {
+                    rotate_config.cur_file_size = metadata.len();
+                }
+
+                Some(Self {
+                    json_format,
+                    buf: BufWriter::new(file),
+                    path: file_path,
+                    file_config,
+                    trader_id,
+                    instance_id,
+                    level: fileout_level,
+                    cur_file_date: Utc::now().date_naive(),
+                })
+            }
             Err(e) => {
                 tracing::error!("Error creating log file: {e}");
                 None
@@ -410,3 +420,72 @@ fn strip_ansi_codes(s: &str) -> String {
     let no_controls = strip_nonprinting_except_newline(s);
     re.replace_all(&no_controls, "").to_string()
 }
+
+////////////////////////////////////////////////////////////////////////////////
+// Tests
+////////////////////////////////////////////////////////////////////////////////
+
+#[cfg(test)]
+mod tests {
+    use log::LevelFilter;
+    use rstest::rstest;
+    use tempfile::tempdir;
+
+    use super::*;
+
+    #[rstest]
+    fn test_file_writer_with_rotation_creates_new_timestamped_file() {
+        let temp_dir = tempdir().unwrap();
+
+        let config = FileWriterConfig {
+            directory: Some(temp_dir.path().to_str().unwrap().to_string()),
+            file_name: Some("test".to_string()),
+            file_format: None,
+            file_rotate: Some(FileRotateConfig::from((2000, 5))),
+        };
+
+        let writer = FileWriter::new(
+            "TRADER-001".to_string(),
+            "instance-123".to_string(),
+            config,
+            LevelFilter::Info,
+        )
+        .unwrap();
+
+        assert_eq!(
+            writer
+                .file_config
+                .file_rotate
+                .as_ref()
+                .unwrap()
+                .cur_file_size,
+            0
+        );
+        assert!(writer.path.to_str().unwrap().contains("test_"));
+    }
+
+    #[rstest]
+    #[case("Hello, World!", "Hello, World!")]
+    #[case("Line1\nLine2", "Line1\nLine2")]
+    #[case("Tab\there", "Tabhere")]
+    #[case("Null\0char", "Nullchar")]
+    #[case("DEL\u{7F}char", "DELchar")]
+    #[case("Bell\u{07}sound", "Bellsound")]
+    #[case("Mix\t\0\u{7F}ed", "Mixed")]
+    fn test_strip_nonprinting_except_newline(#[case] input: &str, #[case] expected: &str) {
+        let result = strip_nonprinting_except_newline(input);
+        assert_eq!(result, expected);
+    }
+
+    #[rstest]
+    #[case("Plain text", "Plain text")]
+    #[case("\x1B[31mRed\x1B[0m", "[31mRed[0m")]
+    #[case("\x1B[1;32mBold Green\x1B[0m", "[1;32mBold Green[0m")]
+    #[case("Before\x1B[0mAfter", "Before[0mAfter")]
+    #[case("\x1B]0;Title\x07Content", "]0;TitleContent")]
+    #[case("Text\t\x1B[31mRed\x1B[0m", "Text[31mRed[0m")]
+    fn test_strip_ansi_codes(#[case] input: &str, #[case] expected: &str) {
+        let result = strip_ansi_codes(input);
+        assert_eq!(result, expected);
+    }
+}

crates/common/src/python/logging.rs

@@ -119,10 +119,12 @@ pub fn py_init_logging(
 ) -> PyResult<LogGuard> {
     let level_file = level_file.map_or(LevelFilter::Off, map_log_level_to_filter);
 
+    let component_levels = parse_component_levels(component_levels).map_err(to_pyvalue_err)?;
+
     let config = LoggerConfig::new(
         map_log_level_to_filter(level_stdout),
         level_file,
-        parse_component_levels(component_levels),
+        component_levels,
         log_components_only.unwrap_or(false),
         is_colored.unwrap_or(true),
         print_config.unwrap_or(false),
@@ -145,18 +147,18 @@ pub fn py_logger_flush() {
 
 fn parse_component_levels(
     original_map: Option<HashMap<String, String>>,
-) -> HashMap<Ustr, LevelFilter> {
+) -> anyhow::Result<HashMap<Ustr, LevelFilter>> {
     match original_map {
         Some(map) => {
             let mut new_map = HashMap::new();
             for (key, value) in map {
                 let ustr_key = Ustr::from(&key);
-                let value = parse_level_filter_str(&value);
-                new_map.insert(ustr_key, value);
+                let level = parse_level_filter_str(&value)?;
+                new_map.insert(ustr_key, level);
             }
-            new_map
+            Ok(new_map)
         }
-        None => HashMap::new(),
+        None => Ok(HashMap::new()),
     }
 }
 

nautilus_trader/common/component.pyx

@@ -1214,7 +1214,7 @@ cpdef LogGuard init_logging(
     str directory = None,
     str file_name = None,
     str file_format = None,
-    dict component_levels: dict[ComponentId, LogLevel] = None,
+    dict component_levels = None,
     bint log_components_only = False,
     bint colors = True,
     bint bypass = False,
@@ -1248,17 +1248,17 @@ cpdef LogGuard init_logging(
         If ``None`` then will write to the current working directory.
     file_name : str, optional
         The custom log file name (will use a '.log' suffix for plain text or '.json' for JSON).
-        If ``None`` will not log to a file (unless `file_auto` is True).
+        If ``None`` will not log to a file.
     file_format : str { 'JSON' }, optional
         The log file format. If ``None`` (default) then will log in plain text.
         If set to 'JSON' then logs will be in JSON format.
-    component_levels : dict[ComponentId, LogLevel]
+    component_levels : dict[str, str], optional
         The additional per component log level filters, where keys are component
-        IDs (e.g. actor/strategy IDs) and values are log levels.
+        IDs as strings (e.g. actor/strategy IDs) and values are log level strings (case-insensitive).
     log_components_only : bool, default False
         If only components with explicit component-level filters should be logged.
         When enabled, only log messages from components that have been explicitly
-        configured in `log_component_levels` will be output.
+        configured in `component_levels` will be output.
     colors : bool, default True
         If ANSI codes should be used to produce colored log lines.
     bypass : bool, default False