diff --git a/src/blueapi/cli/cli.py b/src/blueapi/cli/cli.py index 86a2cd5615..bd2154f0a1 100644 --- a/src/blueapi/cli/cli.py +++ b/src/blueapi/cli/cli.py @@ -26,6 +26,7 @@ from blueapi.client.rest import ( BlueskyRemoteControlError, InvalidParametersError, + NonJsonResponseError, ServiceUnavailableError, UnauthorisedAccessError, UnknownPlanError, @@ -513,13 +514,16 @@ def login(obj: dict) -> None: print("Logged in") except Exception: client = BlueapiClient.from_config(config) - if oidc := client.oidc_config: - auth = SessionManager( - oidc, cache_manager=SessionCacheManager(config.auth_token_path) - ) - auth.start_device_flow() - else: - print("Server is not configured to use authentication!") + try: + if oidc := client.oidc_config: + auth = SessionManager( + oidc, cache_manager=SessionCacheManager(config.auth_token_path) + ) + auth.start_device_flow() + else: + print("Server is not configured to use authentication!") + except NonJsonResponseError as e: + print(str(e)) @main.command(name="logout") diff --git a/src/blueapi/client/rest.py b/src/blueapi/client/rest.py index 095a23afc1..6caa0add20 100644 --- a/src/blueapi/client/rest.py +++ b/src/blueapi/client/rest.py @@ -1,3 +1,4 @@ +import json import logging from collections.abc import Callable, Mapping from typing import Any, Literal, TypeVar @@ -45,6 +46,10 @@ class BlueskyRemoteControlError(Exception): pass +class NonJsonResponseError(Exception): + pass + + class BlueskyRequestError(Exception): def __init__(self, code: int, message: str) -> None: super().__init__(message, code) @@ -109,7 +114,7 @@ def _exception(response: requests.Response) -> Exception | None: if code < 400: return None elif code == 404: - return KeyError(str(response.json())) + return KeyError(str(_response_json(response))) else: return BlueskyRemoteControlError(code, str(response)) @@ -124,7 +129,7 @@ def _create_task_exceptions(response: requests.Response) -> Exception | None: return UnknownPlanError() elif code == 422: try: - content = response.json() + content = _response_json(response) return InvalidParametersError( TypeAdapter(list[ParameterError]).validate_python( content.get("detail", []) @@ -138,6 +143,18 @@ def _create_task_exceptions(response: requests.Response) -> Exception | None: return BlueskyRequestError(code, response.text) +def _response_json(response: requests.Response) -> Any: + try: + return response.json() + except json.decoder.JSONDecodeError as exc: + LOGGER.debug( + f"Invalid json response from <{response.request.url}>: <{response.content}>" + ) + raise NonJsonResponseError( + "Response does not contain a valid JSON object" + ) from exc + + class BlueapiRestClient: _config: RestConfig _session_manager: SessionManager | None @@ -286,7 +303,9 @@ def _request_and_deserialize( f"but client version is {client_version}. " f"Some features may not work as expected." ) - deserialized = TypeAdapter(target_type).validate_python(response.json()) + deserialized = TypeAdapter(target_type).validate_python( + _response_json(response) + ) return deserialized diff --git a/tests/unit_tests/cli/test_cli.py b/tests/unit_tests/cli/test_cli.py index 35bc830ae6..cbde517414 100644 --- a/tests/unit_tests/cli/test_cli.py +++ b/tests/unit_tests/cli/test_cli.py @@ -1138,6 +1138,26 @@ def test_login_with_unauthenticated_server( assert result.exit_code == 0 +@responses.activate +def test_invalid_json( + runner: CliRunner, + config_with_auth: str, + mock_authn_server: responses.RequestsMock, +): + response = responses.add( + responses.GET, + "http://localhost:8000/config/oidc", + body="blah blah", + status=404, + ) + + result = runner.invoke(main, ["-c", config_with_auth, "login"]) + + assert response.call_count == 1 + assert "Response does not contain a valid JSON object\n" == result.output + assert result.exit_code == 0 + + def test_logout_success( runner: CliRunner, config_with_auth: str,