feat: Rust unwrap hardening

rust-bindings/src/expr/errors.rs

@@ -70,6 +70,27 @@ pyo3::create_exception!(
 //      caret column shifted accordingly. Falls back to
 //      `str(self)` for context-free or multi-line errors.
 //
+// Rendering shape of `message_with_expr_prefix` (the "printing"
+// path). When `expr` is attached and single-line, it produces a
+// three-line, caret-annotated message:
+//
+//     <base message>
+//       <prefix><expr>
+//               ^
+//
+//   * line 1 is `self._base_message` (the original message text);
+//   * line 2 is the expression source, two-space-indented, with
+//     the caller-supplied `prefix` prepended
+//     (`"  " + prefix + expr`);
+//   * line 3 is emitted only when `col_offset is not None`: a
+//     caret under the offending column. The caret is shifted right
+//     by `len(prefix)` (`" " * (col_offset + len(prefix))`) so it
+//     stays aligned with the source character after the prefix
+//     pushes the expression text rightward.
+//
+// If `expr is None` or the expression is multi-line (`"\n" in
+// expr`), it returns plain `str(self)` with no caret annotation.
+//
 // Implementing these as `#[pyfunction]`s installed via `setattr`
 // would *almost* work, except that PyO3's `#[pyfunction]` builds
 // a `PyCFunction` (builtin function) which is not bound to its

rust-bindings/src/expr/symbol_table.rs

@@ -73,10 +73,10 @@ impl PySymbolTable {
         use pyo3::IntoPyObjectExt;
         match self.inner.get(key) {
             Some(openjd_expr::symbol_table::SymbolTableEntry::Value(v)) => {
-                Ok(PyExprValue { inner: v.clone() }.into_py_any(py).unwrap())
+                PyExprValue { inner: v.clone() }.into_py_any(py)
             }
             Some(openjd_expr::symbol_table::SymbolTableEntry::Table(t)) => {
-                Ok(PySymbolTable { inner: t.clone() }.into_py_any(py).unwrap())
+                PySymbolTable { inner: t.clone() }.into_py_any(py)
             }
             None => Err(pyo3::exceptions::PyKeyError::new_err(key.to_string())),
         }
@@ -85,12 +85,12 @@ impl PySymbolTable {
     fn get(&self, py: Python<'_>, name: &str) -> PyResult<Option<Py<pyo3::PyAny>>> {
         use pyo3::IntoPyObjectExt;
         match self.inner.get(name) {
-            Some(openjd_expr::symbol_table::SymbolTableEntry::Value(v)) => Ok(Some(
-                PyExprValue { inner: v.clone() }.into_py_any(py).unwrap(),
-            )),
-            Some(openjd_expr::symbol_table::SymbolTableEntry::Table(t)) => Ok(Some(
-                PySymbolTable { inner: t.clone() }.into_py_any(py).unwrap(),
-            )),
+            Some(openjd_expr::symbol_table::SymbolTableEntry::Value(v)) => {
+                Ok(Some(PyExprValue { inner: v.clone() }.into_py_any(py)?))
+            }
+            Some(openjd_expr::symbol_table::SymbolTableEntry::Table(t)) => {
+                Ok(Some(PySymbolTable { inner: t.clone() }.into_py_any(py)?))
+            }
             None => Ok(None),
         }
     }

rust-bindings/src/model/step_dependency_graph.rs

@@ -18,37 +18,45 @@ pub(crate) struct PyStepDependencyGraph {
 }
 
 impl PyStepDependencyGraph {
-    fn make_node(&self, node_index: usize) -> PyStepDependencyNode {
-        let node = self.inner.node(node_index).unwrap();
+    fn make_node(&self, node_index: usize) -> PyResult<PyStepDependencyNode> {
+        let node = self.inner.node(node_index).ok_or_else(|| {
+            pyo3::exceptions::PyIndexError::new_err(format!(
+                "step dependency graph has no node at index {node_index}"
+            ))
+        })?;
+        let step_for = |idx: usize| -> PyResult<openjd_model::job::Step> {
+            self.job_steps.get(idx).cloned().ok_or_else(|| {
+                pyo3::exceptions::PyIndexError::new_err(format!(
+                    "step dependency edge references out-of-range step index {idx}"
+                ))
+            })
+        };
+        let edge_for = |edge_idx| -> PyResult<Option<PyStepDependencyEdge>> {
+            match self.inner.edge(edge_idx) {
+                Some(edge) => Ok(Some(PyStepDependencyEdge {
+                    origin_step: step_for(edge.origin)?,
+                    dependent_step: step_for(edge.dependent)?,
+                })),
+                None => Ok(None),
+            }
+        };
         let in_edges: Vec<PyStepDependencyEdge> = node
             .in_edges
             .iter()
-            .filter_map(|&edge_idx| {
-                let edge = self.inner.edge(edge_idx)?;
-                Some(PyStepDependencyEdge {
-                    origin_step: self.job_steps[edge.origin].clone(),
-                    dependent_step: self.job_steps[edge.dependent].clone(),
-                })
-            })
-            .collect();
+            .filter_map(|&edge_idx| edge_for(edge_idx).transpose())
+            .collect::<PyResult<_>>()?;
         let out_edges: Vec<PyStepDependencyEdge> = node
             .out_edges
             .iter()
-            .filter_map(|&edge_idx| {
-                let edge = self.inner.edge(edge_idx)?;
-                Some(PyStepDependencyEdge {
-                    origin_step: self.job_steps[edge.origin].clone(),
-                    dependent_step: self.job_steps[edge.dependent].clone(),
-                })
-            })
-            .collect();
-        PyStepDependencyNode {
+            .filter_map(|&edge_idx| edge_for(edge_idx).transpose())
+            .collect::<PyResult<_>>()?;
+        Ok(PyStepDependencyNode {
             step: PyStep {
-                inner: self.job_steps[node.step_index].clone(),
+                inner: step_for(node.step_index)?,
             },
             in_edges,
             out_edges,
-        }
+        })
     }
 }
 
@@ -66,7 +74,7 @@ impl PyStepDependencyGraph {
     }
 
     #[getter]
-    fn _nodes(&self) -> Vec<PyStepDependencyNode> {
+    fn _nodes(&self) -> PyResult<Vec<PyStepDependencyNode>> {
         (0..self.inner.node_count())
             .map(|i| self.make_node(i))
             .collect()
@@ -76,17 +84,25 @@ impl PyStepDependencyGraph {
         let node = self.inner.step_node(stepname).ok_or_else(|| {
             pyo3::exceptions::PyKeyError::new_err(format!("No step named '{stepname}'"))
         })?;
-        Ok(self.make_node(node.step_index))
+        self.make_node(node.step_index)
     }
 
     fn topo_sorted(&self) -> PyResult<Vec<PyStep>> {
         let indices = self.inner.topo_sorted().map_err(model_err_to_py)?;
-        Ok(indices
+        indices
             .into_iter()
-            .map(|i| PyStep {
-                inner: self.job_steps[i].clone(),
+            .map(|i| {
+                self.job_steps
+                    .get(i)
+                    .cloned()
+                    .map(|inner| PyStep { inner })
+                    .ok_or_else(|| {
+                        pyo3::exceptions::PyIndexError::new_err(format!(
+                            "topo sort produced out-of-range step index {i}"
+                        ))
+                    })
             })
-            .collect())
+            .collect()
     }
 
     fn step_names(&self) -> PyResult<Vec<String>> {

rust-bindings/src/model/step_param_space.rs

@@ -91,6 +91,15 @@ fn param_type_to_expr_type(pt: TaskParameterType) -> openjd_expr::ExprType {
     }
 }
 
+/// Lock the iterator mutex, recovering the guard if a previous holder
+/// panicked. A panic inside `next()` / `contains()` / `validate_containment()`
+/// would otherwise poison the mutex, and every subsequent `.lock().unwrap()`
+/// would raise an uncatchable `PanicException` on the Python side — wedging
+/// the iterator object permanently. Recovering keeps it usable.
+fn lock_recover<T>(m: &Mutex<T>) -> std::sync::MutexGuard<'_, T> {
+    m.lock().unwrap_or_else(|poisoned| poisoned.into_inner())
+}
+
 #[cfg_attr(feature = "stub-gen", gen_stub_pyclass(module = "openjd._openjd_rs"))]
 #[pyclass(module = "openjd.model._v1.job", name = "StepParameterSpaceIterator")]
 pub(crate) struct PyStepParameterSpaceIterator {
@@ -154,7 +163,7 @@ impl PyStepParameterSpaceIterator {
         // Match the pure-Python reference: adaptive-chunked spaces
         // cannot answer `len()` because the count depends on the
         // dynamic chunk size that may change during execution.
-        let iter = self.iter.lock().unwrap();
+        let iter = lock_recover(&self.iter);
         if iter.chunks_adaptive() {
             return Err(pyo3::exceptions::PyValueError::new_err(
                 "Length is not available because the parameter space uses adaptive chunking.",
@@ -195,7 +204,7 @@ impl PyStepParameterSpaceIterator {
     }
 
     fn __next__(&self, py: Python<'_>) -> PyResult<Option<Py<PyDict>>> {
-        let mut iter = self.iter.lock().unwrap();
+        let mut iter = lock_recover(&self.iter);
         match iter.next() {
             Some(params) => Ok(Some(task_param_set_to_py(py, &params)?)),
             None => Ok(None),
@@ -204,7 +213,7 @@ impl PyStepParameterSpaceIterator {
 
     fn __contains__(&self, item: &Bound<'_, PyDict>) -> PyResult<bool> {
         let params = extract_task_parameter_set(item)?;
-        let iter = self.iter.lock().unwrap();
+        let iter = lock_recover(&self.iter);
         Ok(iter.contains(&params))
     }
 
@@ -217,13 +226,13 @@ impl PyStepParameterSpaceIterator {
     /// crate's ``StepParameterSpaceIterator::validate_containment``.
     fn validate_containment(&self, params: &Bound<'_, PyDict>) -> PyResult<()> {
         let params = extract_task_parameter_set(params)?;
-        let iter = self.iter.lock().unwrap();
+        let iter = lock_recover(&self.iter);
         iter.validate_containment(&params)
             .map_err(pyo3::exceptions::PyValueError::new_err)
     }
 
     fn reset_iter(&self) {
-        let mut iter = self.iter.lock().unwrap();
+        let mut iter = lock_recover(&self.iter);
         iter.reset();
     }
 
@@ -234,19 +243,19 @@ impl PyStepParameterSpaceIterator {
 
     #[getter]
     fn chunks_adaptive(&self) -> bool {
-        let iter = self.iter.lock().unwrap();
+        let iter = lock_recover(&self.iter);
         iter.chunks_adaptive()
     }
 
     #[getter]
     fn chunks_parameter_name(&self) -> Option<String> {
-        let iter = self.iter.lock().unwrap();
+        let iter = lock_recover(&self.iter);
         iter.chunks_parameter_name().map(|s| s.to_string())
     }
 
     #[getter]
     fn chunks_default_task_count(&self) -> Option<usize> {
-        let iter = self.iter.lock().unwrap();
+        let iter = lock_recover(&self.iter);
         iter.chunks_default_task_count()
     }
 
@@ -257,7 +266,7 @@ impl PyStepParameterSpaceIterator {
                 "chunks_default_task_count must be a positive integer.",
             ));
         }
-        let mut iter = self.iter.lock().unwrap();
+        let mut iter = lock_recover(&self.iter);
         if !iter.chunks_adaptive() {
             return Err(pyo3::exceptions::PyValueError::new_err(
                 "The parameter space does not use adaptive chunking, so cannot modify chunks_default_task_count.",