diff --git a/src/blueapi/service/runner.py b/src/blueapi/service/runner.py index 734d24bf8..2b5a5f37f 100644 --- a/src/blueapi/service/runner.py +++ b/src/blueapi/service/runner.py @@ -30,6 +30,8 @@ P = ParamSpec("P") T = TypeVar("T") +BLANK_REPORT = "The source message was blank" + def _init_worker(): # Replace sigint to allow subprocess to be terminated @@ -80,12 +82,12 @@ def start(self): initialized=True, ) except Exception as e: + LOGGER.exception(e) self._state = EnvironmentResponse( environment_id=environment_id, initialized=False, - error_message=str(e), + error_message=_safe_exception_message(e), ) - LOGGER.exception(e) @start_as_current_span(TRACER) def stop(self): @@ -101,12 +103,12 @@ def stop(self): error_message=self._state.error_message, ) except Exception as e: + LOGGER.exception(e) self._state = EnvironmentResponse( environment_id=environment_id, initialized=False, - error_message=str(e), + error_message=_safe_exception_message(e), ) - LOGGER.exception(e) @start_as_current_span(TRACER, "function", "args", "kwargs") def run( @@ -188,3 +190,11 @@ def _validate_function(func: Any, function_name: str) -> Callable: elif not callable(func): raise RpcError(f"{function_name}: Object in subprocess is not a function") return func + + +def _safe_exception_message(e: Exception) -> str: + return _safe_message(type(e).__name__, str(e)) + + +def _safe_message(owner: str, message: str | None) -> str: + return f"{owner}: {BLANK_REPORT if (not message or message.isspace()) else message}" diff --git a/tests/unit_tests/service/test_runner.py b/tests/unit_tests/service/test_runner.py index 0cdfe477a..f96fa3826 100644 --- a/tests/unit_tests/service/test_runner.py +++ b/tests/unit_tests/service/test_runner.py @@ -17,6 +17,7 @@ InvalidRunnerStateError, RpcError, WorkerDispatcher, + _safe_exception_message, import_and_run_function, ) @@ -71,19 +72,33 @@ def test_raises_if_used_before_started(runner: WorkerDispatcher): runner.run(interface.get_plans) -def test_error_on_runner_setup(runner: WorkerDispatcher, mock_subprocess: Mock): - error_message = "Intentional start_worker exception" - environment_id = uuid.uuid4() - expected_state = EnvironmentResponse( - environment_id=environment_id, - initialized=False, - error_message=error_message, +@pytest.mark.parametrize( + "message", + [ + None, + "", + " ", + "Intentional start_worker exception", + ], +) +def test_using_safe_exception_message_copes_with_all_message_types_on_runner_setup( + runner: WorkerDispatcher, mock_subprocess: Mock, message: str | None +): + try: + raise Exception() if message is None else Exception(message) + except Exception as e: + expected_state = EnvironmentResponse( + environment_id=uuid.uuid4(), + initialized=False, + error_message=_safe_exception_message(e), + ) + mock_subprocess.apply.side_effect = ( + Exception() if message is None else Exception(message) ) - mock_subprocess.apply.side_effect = Exception(error_message) - # Calling reload here instead of start also indirectly - # tests that stop() doesn't raise if there is no error message - # and the runner is not yet initialised + # Calling reload here instead of start also indirectly tests + # that stop() doesn't raise if there is no error message and the + # runner is not yet initialised. runner.reload() state = runner.state expected_state.environment_id = state.environment_id @@ -116,7 +131,9 @@ def test_can_reload_after_an_error(pool_mock: MagicMock): runner.start() current_env = runner.state.environment_id assert runner.state == EnvironmentResponse( - environment_id=current_env, initialized=False, error_message="invalid code" + environment_id=current_env, + initialized=False, + error_message="SyntaxError: invalid code", ) runner.reload()