fix: improve error handling when request receives non-JSON response (#1477)

improve error handling when request receives non-JSON response.

Closes 1472

Author: NeilSmithDLS

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")

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
 
 

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,