[RSDK-8127] on invalid config create empty robot over panicking (#274)

viamrobotics · Jul 24, 2024 · 6b2622d · 6b2622d
1 parent d5f7e23
commit 6b2622d
Show file tree

Hide file tree

Showing 3 changed files with 22 additions and 42 deletions.
diff --git a/micro-rdk/src/common/board.rs b/micro-rdk/src/common/board.rs
@@ -44,6 +44,8 @@ pub enum BoardError {
     #[error(transparent)]
     #[cfg(feature = "esp32")]
     EspError(#[from] EspError),
+    #[error("construction error test")]
+    TestError,
 }
 
 pub static COMPONENT_NAME: &str = "board";
@@ -130,6 +132,10 @@ impl FakeBoard {
     }
 
     pub(crate) fn from_config(cfg: ConfigType) -> Result<BoardType, BoardError> {
+        if cfg.get_attribute::<bool>("fail_new").unwrap_or(false) {
+            return Err(BoardError::TestError);
+        }
+
         let analogs = if let Ok(analog_confs) = cfg.get_attribute::<HashMap<&str, f64>>("analogs") {
             analog_confs
                 .iter()

diff --git a/micro-rdk/src/esp32/entry.rs b/micro-rdk/src/esp32/entry.rs
@@ -103,36 +103,23 @@ pub async fn serve_web_inner<S>(
         RobotRepresentation::WithRobot(robot) => Arc::new(Mutex::new(robot)),
         RobotRepresentation::WithRegistry(registry) => {
             log::info!("building robot from config");
-            let r = match LocalRobot::from_cloud_config(
+            let (r, err) = match LocalRobot::from_cloud_config(
                 exec.clone(),
                 app_config.get_robot_id(),
                 &cfg_response,
                 registry,
                 cfg_received_datetime,
             ) {
-                Ok(robot) => {
-                    if let Some(datetime) = cfg_received_datetime {
-                        let logs = vec![config_log_entry(datetime, None)];
-                        app_client
-                            .push_logs(logs)
-                            .await
-                            .expect("could not push logs to app");
-                    }
-                    robot
-                }
+                Ok(robot) => (robot, None),
                 Err(err) => {
-                    if let Some(datetime) = cfg_received_datetime {
-                        let logs = vec![config_log_entry(datetime, Some(err))];
-                        app_client
-                            .push_logs(logs)
-                            .await
-                            .expect("could not push logs to app");
-                    }
-                    // TODO(RSDK-8172) shouldn't panic here, when we support offline mode and
-                    // reloading configuration this should be removed
-                    panic!("couldn't build robot");
+                    log::error!("could not build robot from config due to {:?}, defaulting to empty robot until a valid config is accessible", err);
+                    (LocalRobot::new(), Some(err))
                 }
             };
+            if let Some(datetime) = cfg_received_datetime {
+                let logs = vec![config_log_entry(datetime, err)];
+                let _ = app_client.push_logs(logs).await;
+            }
             Arc::new(Mutex::new(r))
         }
     };

diff --git a/micro-rdk/src/native/entry.rs b/micro-rdk/src/native/entry.rs
@@ -73,36 +73,23 @@ pub async fn serve_web_inner<S>(
         RobotRepresentation::WithRobot(robot) => Arc::new(Mutex::new(robot)),
         RobotRepresentation::WithRegistry(registry) => {
             log::info!("building robot from config");
-            let r = match LocalRobot::from_cloud_config(
+            let (r, err) = match LocalRobot::from_cloud_config(
                 exec.clone(),
                 app_config.get_robot_id(),
                 &cfg_response,
                 registry,
                 cfg_received_datetime,
             ) {
-                Ok(robot) => {
-                    if let Some(datetime) = cfg_received_datetime {
-                        let logs = vec![config_log_entry(datetime, None)];
-                        app_client
-                            .push_logs(logs)
-                            .await
-                            .expect("could not push logs to app");
-                    }
-                    robot
-                }
+                Ok(robot) => (robot, None),
                 Err(err) => {
-                    if let Some(datetime) = cfg_received_datetime {
-                        let logs = vec![config_log_entry(datetime, Some(err))];
-                        app_client
-                            .push_logs(logs)
-                            .await
-                            .expect("could not push logs to app");
-                    }
-                    // TODO(RSDK-8127) shouldn't panic here, when we support offline mode and
-                    // reloading configuration this should be removed
-                    panic!("couldn't build robot");
+                    log::error!("could not build robot from config due to {:?}, defaulting to empty robot until a valid config is accessible", err);
+                    (LocalRobot::new(), Some(err))
                 }
             };
+            if let Some(datetime) = cfg_received_datetime {
+                let logs = vec![config_log_entry(datetime, err)];
+                let _ = app_client.push_logs(logs).await;
+            }
             Arc::new(Mutex::new(r))
         }
     };