Skip to content

Commit

Permalink
Acqp-323 prevent blank connection errors in STOMP connections (#803)
Browse files Browse the repository at this point in the history
Add handler functions to check for Exceptions with message that are
None, Empty String or a String that consists only of spaces

Fixes #792
  • Loading branch information
keithralphs authored Jan 28, 2025
1 parent 07bd6ba commit e18b9be
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 16 deletions.
18 changes: 14 additions & 4 deletions src/blueapi/service/runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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):
Expand All @@ -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(
Expand Down Expand Up @@ -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}"
41 changes: 29 additions & 12 deletions tests/unit_tests/service/test_runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
InvalidRunnerStateError,
RpcError,
WorkerDispatcher,
_safe_exception_message,
import_and_run_function,
)

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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()
Expand Down

0 comments on commit e18b9be

Please sign in to comment.