diff --git a/crates/sqltide-python/src/sensors/providers.rs b/crates/sqltide-python/src/sensors/providers.rs index 9417dc9f..ac7e8dba 100644 --- a/crates/sqltide-python/src/sensors/providers.rs +++ b/crates/sqltide-python/src/sensors/providers.rs @@ -170,10 +170,8 @@ fn load_provider_module<'py>(file_path: &Any<'py>, project_dir: &Any<'py>) -> Py match message { Ok(message) => Err(discovery_error(py, message, Some(error))), Err(failed) => { - let failure: &Bound<'_, pyo3::exceptions::PyBaseException> = failed.value(py); - if failure.getattr("__context__")?.is_none() { - failure.setattr("__context__", error.value(py))?; - } + // As in an `except` block, the import error is the context. + failed.value(py).setattr("__context__", error.value(py))?; Err(failed) } } diff --git a/crates/sqltide-python/src/sensors/registry.rs b/crates/sqltide-python/src/sensors/registry.rs index 95243655..f9c79144 100644 --- a/crates/sqltide-python/src/sensors/registry.rs +++ b/crates/sqltide-python/src/sensors/registry.rs @@ -18,7 +18,8 @@ const DEFAULT_SENSOR_TIMEOUT_SECONDS: f64 = 60.0; type Any<'py> = Bound<'py, PyAny>; -/// `SensorError(message, location=..., related_locations=...)`, chained to `cause` if given. +/// `SensorError(message, location=..., related_locations=...)` raised `from cause` when one is +/// given, otherwise chained to the exception being handled, as a Python `raise` is. fn located_error( py: Python<'_>, message: String, @@ -38,8 +39,14 @@ fn located_error( match built() { Ok(error) => { let error: PyErr = PyErr::from_value(error); - if let Some(cause) = cause { - error.set_cause(py, Some(cause)); + let Some(cause) = cause else { + return chained_to_handled(py, error); + }; + // `raise ... from cause` inside `except` sets both the cause and the context. + let context: Any<'_> = cause.value(py).clone().into_any(); + error.set_cause(py, Some(cause)); + if let Err(failed) = error.value(py).setattr("__context__", context) { + return failed; } error } @@ -162,9 +169,30 @@ fn load_sensor_module<'py>(file_path: &Any<'py>, project_dir: &Any<'py>) -> PyRe Ok(()) })(); let restored: PyResult<()> = sys.setattr("path", old_path); - let outcome: PyResult<()> = match loaded { - Ok(()) => Ok(()), - Err(error) if error.is_instance_of::(py) => { + let outcome: PyResult<()> = loaded.map_err(|error| load_failure(file_path, error)); + outcome?; + restored?; + Ok(module) +} + +/// The error to raise when a sensor file failed to import. As in an `except` block, a failure +/// while describing the import error has it as context. +fn load_failure(file_path: &Any<'_>, error: PyErr) -> PyErr { + let py: Python<'_> = file_path.py(); + let original: Any<'_> = error.value(py).clone().into_any(); + match described_load_failure(file_path, error) { + Ok(described) => described, + Err(failed) => match failed.value(py).setattr("__context__", original) { + Ok(()) => failed, + Err(set) => set, + }, + } +} + +fn described_load_failure<'py>(file_path: &Any<'py>, error: PyErr) -> PyResult { + let py: Python<'py> = file_path.py(); + Ok(match error { + error if error.is_instance_of::(py) => { let value: Any<'py> = error.value(py).clone().into_any(); let location: Any<'py> = source_location( py, @@ -172,22 +200,20 @@ fn load_sensor_module<'py>(file_path: &Any<'py>, project_dir: &Any<'py>) -> PyRe or_one(value.getattr("lineno")?)?, or_one(value.getattr("offset")?)?, )?; - Err(located_error( + located_error( py, format!("Failed to load {}: {}", text(file_path)?, text(&value)?), Some(location), Vec::new(), Some(error), - )) - } - Err(error) if error.is_instance(py, &attr(py, SENSOR_EXCEPTIONS, "SensorError")?) => { - Err(error) + ) } - Err(error) if error.is_instance_of::(py) => { + error if error.is_instance(py, &attr(py, SENSOR_EXCEPTIONS, "SensorError")?) => error, + error if error.is_instance_of::(py) => { let line: Any<'py> = exception_line(py, &error, file_path)?; let location: Any<'py> = source_location(py, file_path, line, 1_i64.into_pyobject(py)?.into_any())?; - Err(located_error( + located_error( py, format!( "Failed to load {}: {}", @@ -197,13 +223,10 @@ fn load_sensor_module<'py>(file_path: &Any<'py>, project_dir: &Any<'py>) -> PyRe Some(location), Vec::new(), Some(error), - )) + ) } - Err(error) => Err(error), - }; - outcome?; - restored?; - Ok(module) + error => error, + }) } /// The first paragraph of the handler's docstring, or `None`. @@ -584,21 +607,15 @@ fn polling_sensor<'py>( fn validate_sensor_retry_policy(max_attempts: Any<'_>, backoff_seconds: Any<'_>) -> PyResult<()> { let py: Python<'_> = max_attempts.py(); if max_attempts.lt(1)? { - return Err(chained_to_handled( + return Err(plain_error( py, - plain_error( - py, - "SensorRetryPolicy.max_attempts must be at least 1".to_owned(), - ), + "SensorRetryPolicy.max_attempts must be at least 1".to_owned(), )); } if backoff_seconds.lt(0)? { - return Err(chained_to_handled( + return Err(plain_error( py, - plain_error( - py, - "SensorRetryPolicy.backoff_seconds must not be negative".to_owned(), - ), + "SensorRetryPolicy.backoff_seconds must not be negative".to_owned(), )); } Ok(()) diff --git a/tests/unit/src/sqltide/sensors/main/_test_types.py b/tests/unit/src/sqltide/sensors/main/_test_types.py index 396eca91..5af77a37 100644 --- a/tests/unit/src/sqltide/sensors/main/_test_types.py +++ b/tests/unit/src/sqltide/sensors/main/_test_types.py @@ -42,6 +42,15 @@ class CompileSensorsProviderPathTestCase: expected_context_type_name: str +@dataclass(frozen=True) +class CompileSensorsLoadContextTestCase: + description: str + sensor_files: tuple[tuple[str, str], ...] + expected_error_type: type[BaseException] + expected_context_type_name: str + expected_context_is_cause: bool + + @dataclass(frozen=True) class CompileSensorsProviderReloadTestCase: description: str diff --git a/tests/unit/src/sqltide/sensors/main/helpers.py b/tests/unit/src/sqltide/sensors/main/helpers.py index 6a9aa99a..fb9cfcc6 100644 --- a/tests/unit/src/sqltide/sensors/main/helpers.py +++ b/tests/unit/src/sqltide/sensors/main/helpers.py @@ -75,6 +75,10 @@ class OrdersWebhook(Provider): orders_webhook_timeout: float """ +FAILING_SENSOR_SOURCE: str = """ +raise RuntimeError("sensor module failed") +""" + UNPRINTABLE_ERROR_PROVIDER_SOURCE: str = """ class UnprintableError(Exception): def __str__(self) -> str: diff --git a/tests/unit/src/sqltide/sensors/main/test_compile_sensors.py b/tests/unit/src/sqltide/sensors/main/test_compile_sensors.py index e9aa1032..ac3e5615 100644 --- a/tests/unit/src/sqltide/sensors/main/test_compile_sensors.py +++ b/tests/unit/src/sqltide/sensors/main/test_compile_sensors.py @@ -14,6 +14,7 @@ from tests.unit.src.sqltide.sensors.main._test_types import ( CompileSensorsErrorTestCase, CompileSensorsEventTypeTestCase, + CompileSensorsLoadContextTestCase, CompileSensorsProviderErrorTestCase, CompileSensorsProviderPathTestCase, CompileSensorsProviderReloadTestCase, @@ -23,6 +24,7 @@ BROKEN_PROVIDER_SOURCE, CATALOG_EVENT_SENSORS_SOURCE, DUPLICATE_PROVIDER_SOURCE, + FAILING_SENSOR_SOURCE, INVALID_NAME_PROVIDER_SOURCE, LOOKALIKE_EVENT_SENSOR_SOURCE, MIXED_PROVIDER_SOURCE, @@ -283,6 +285,42 @@ def test_given_unprintable_provider_import_error_when_compiling_then_sys_path_is assert type(error_info.value.__context__).__name__ == test_case.expected_context_type_name +@pytest.mark.parametrize( + "test_case", + [ + CompileSensorsLoadContextTestCase( + description="a failed sensor import is both the cause and the context", + sensor_files=(("sensors/failing.py", FAILING_SENSOR_SOURCE),), + expected_error_type=SensorError, + expected_context_type_name="RuntimeError", + expected_context_is_cause=True, + ), + CompileSensorsLoadContextTestCase( + description="an import error whose message cannot be rendered is the context", + sensor_files=(("sensors/unprintable.py", UNPRINTABLE_ERROR_PROVIDER_SOURCE),), + expected_error_type=ZeroDivisionError, + expected_context_type_name="UnprintableError", + expected_context_is_cause=False, + ), + ], + ids=lambda case: case.description, +) +def test_given_failing_sensor_import_when_compiling_then_errors_chain_as_python_does( + test_case: CompileSensorsLoadContextTestCase, tmp_path: Path +) -> None: + write_project_files(project_dir=tmp_path, files=test_case.sensor_files) + path_before: list[str] = list(sys.path) + + with pytest.raises(test_case.expected_error_type) as error_info: + _ = compile_sensors(project_dir=tmp_path, reserved_names=frozenset()) + + assert sys.path == path_before + assert type(error_info.value.__context__).__name__ == test_case.expected_context_type_name + assert ( + error_info.value.__context__ is error_info.value.__cause__ + ) is test_case.expected_context_is_cause + + @pytest.mark.parametrize( "test_case", [