chore: improve default error handling 1379 1409

.github/workflows/asyncapi.yml

@@ -9,6 +9,8 @@ on:
     tags:
       - '*'
   pull_request:
+  merge_group:
+    types: [checks_requested]
 
 jobs:
   validate:

.github/workflows/backstage.yml

@@ -7,6 +7,8 @@ on:
     tags:
       - '*'
   pull_request:
+  merge_group:
+    types: [checks_requested]
 
 jobs:
   validate:

.github/workflows/ci.yml

@@ -7,6 +7,8 @@ on:
     tags:
       - "*"
   pull_request:
+  merge_group:
+    types: [checks_requested]
 
 jobs:
   lint:

.github/workflows/codeql.yml

@@ -18,7 +18,8 @@ on:
     tags:
       - '*'
   pull_request:
-
+  merge_group:
+    types: [checks_requested]
 
 jobs:
   analyze:
@@ -61,7 +62,7 @@ jobs:
 
       # Initializes the CodeQL tools for scanning.
       - name: Initialize CodeQL
-        uses: github/codeql-action/init@c793b717bc78562f491db7b0e93a3a178b099162 # v4
+        uses: github/codeql-action/init@c10b8064de6f491fea524254123dbe5e09572f13 # v4
         with:
           languages: ${{ matrix.language }}
           build-mode: ${{ matrix.build-mode }}
@@ -88,6 +89,6 @@ jobs:
           echo '  make release'
           exit 1
       - name: Perform CodeQL Analysis
-        uses: github/codeql-action/analyze@c793b717bc78562f491db7b0e93a3a178b099162 # v4
+        uses: github/codeql-action/analyze@c10b8064de6f491fea524254123dbe5e09572f13 # v4
         with:
           category: "/language:${{matrix.language}}"

.github/workflows/pr.yml

@@ -3,13 +3,15 @@ name: PR Conventional Commit Validation
 on:
   pull_request:
     types: [opened, synchronize, reopened, edited]
+  merge_group:
+    types: [checks_requested]
 
 jobs:
   validate-pr-title:
     runs-on: ubuntu-latest
     steps:
       - name: PR Conventional Commit Validation
-        uses:  ytanikin/pr-conventional-commits@1.5.1
+        uses: ytanikin/pr-conventional-commits@1.5.2
         with:
           task_types: '["feat","fix","docs","test","ci","refactor","perf","chore","revert", "depr"]'
-          add_label: 'false'
+          add_label: "false"

docs/tutorials/scripting-plans.md

@@ -160,6 +160,8 @@ This will be one of `WorkerEvent`, `ProgressEvent` or `DataEvent`.
 An example that prints data for each point could be something like
 
 ```python
+from blueapi.core.bluesky_types import DataEvent
+
 def feedback(evt):
     match evt:
         case DataEvent(name="start"):

helm/blueapi/templates/statefulset.yaml

@@ -83,6 +83,10 @@ spec:
       {{- if .Values.initContainer.enabled }}
       initContainers:
       - name: setup-scratch
+        {{- with .Values.securityContext }}
+        securityContext:
+          {{- toYaml . | nindent 10 }}
+        {{- end }}
         image: "{{ .Values.image.repository }}:{{ .Values.image.tag | default .Chart.AppVersion }}"
         imagePullPolicy: {{ .Values.image.pullPolicy }}
         resources:

src/blueapi/cli/cli.py

@@ -26,6 +26,7 @@
 from blueapi.client.rest import (
     BlueskyRemoteControlError,
     InvalidParametersError,
+    NonJsonResponseError,
     ServiceUnavailableError,
     UnauthorisedAccessError,
     UnknownPlanError,
@@ -83,8 +84,24 @@ def is_str_dict(val: Any) -> TypeGuard[TaskParameters]:
 @click.option(
     "-c", "--config", type=Path, help="Path to configuration YAML file", multiple=True
 )
+@click.option(
+    "-v",
+    "--verbose",
+    "log_level",
+    flag_value="DEBUG",
+    help="Include DEBUG level logging output",
+)
+@click.option(
+    "-q",
+    "--quiet",
+    "log_level",
+    flag_value="ERROR",
+    help="Reduce logging noise to only show errors",
+)
 @click.pass_context
-def main(ctx: click.Context, config: tuple[Path, ...]) -> None:
+def main(
+    ctx: click.Context, config: tuple[Path, ...], log_level: str | None = None
+) -> None:
     # if no command is supplied, run with the options passed
 
     # Set umask to DLS standard
@@ -96,6 +113,9 @@ def main(ctx: click.Context, config: tuple[Path, ...]) -> None:
     except FileNotFoundError as fnfe:
         raise ClickException(f"Config file not found: {fnfe.filename}") from fnfe
 
+    if log_level:
+        config_loader.use_values({"logging": {"level": log_level}})
+
     loaded_config: ApplicationConfig = config_loader.load()
 
     set_up_logging(loaded_config.logging)
@@ -493,13 +513,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,5 @@
+import json
+import logging
 from collections.abc import Callable, Mapping
 from typing import Any, Literal, TypeVar
 
@@ -10,6 +12,7 @@
 )
 from pydantic import BaseModel, TypeAdapter, ValidationError
 
+from blueapi import __version__
 from blueapi.config import RestConfig
 from blueapi.service.authentication import JWTAuth, SessionManager
 from blueapi.service.model import (
@@ -32,25 +35,44 @@
 
 TRACER = get_tracer("rest")
 
+LOGGER = logging.getLogger(__name__)
+
 
 class BlueskyRequestError(Exception):
+    """Error response from the blueapi server.(422,450,500)"""
+
     def __init__(self, code: int | None = None, message: str = "") -> None:
         super().__init__(code, message)
 
 
-class UnauthorisedAccessError(BlueskyRequestError):
-    pass
+class UnauthorisedAccessError(Exception):
+    """Request was rejected due to missing or invalid credentials (401/403)."""
+
+    def __init__(self, code: int | None = None, message: str = "") -> None:
+        super().__init__(code, message)
 
 
 class BlueskyRemoteControlError(Exception):
+    """Failure communicating with the blueapi server (e.g. connection refused)."""
+
+    pass
+
+
+class NonJsonResponseError(Exception):
+    """Server returned a response that could not be parsed as JSON."""
+
     pass
 
 
 class NotFoundError(BlueskyRequestError):
+    """Requested something that couldn't be found (404)."""
+
     pass
 
 
 class UnknownPlanError(BlueskyRequestError):
+    """ "Plan '{name}' was not recognised" """
+
     pass
 
 
@@ -113,7 +135,14 @@ def _exception(response: requests.Response) -> Exception | None:
     elif code == 404:
         return NotFoundError(code, response.text)
     else:
-        return BlueskyRemoteControlError(response.text)
+        try:
+            body = _response_json(response)
+            message = (body.get("detail") if isinstance(body, dict) else None) or (
+                response.text
+            )
+        except NonJsonResponseError:
+            message = response.text
+        return BlueskyRemoteControlError(code, message)
 
 
 def _create_task_exceptions(response: requests.Response) -> Exception | None:
@@ -126,7 +155,7 @@ def _create_task_exceptions(response: requests.Response) -> Exception | None:
         return UnknownPlanError(code, response.text)
     elif code == 422:
         try:
-            content = response.json()
+            content = _response_json(response)
             return InvalidParametersError(
                 TypeAdapter(list[ParameterError]).validate_python(
                     content.get("detail", [])
@@ -140,6 +169,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
@@ -277,7 +318,20 @@ def _request_and_deserialize(
             raise exception
         if response.status_code == status.HTTP_204_NO_CONTENT:
             raise NoContentError(target_type)
-        deserialized = TypeAdapter(target_type).validate_python(response.json())
+        if (server_version := response.headers.get("x-blueapi-version")) is not None:
+            from packaging.version import Version
+
+            if (server_version := Version(server_version).base_version) != (
+                client_version := Version(__version__).base_version
+            ):
+                LOGGER.warning(
+                    f"Version mismatch: Blueapi server version is {server_version} "
+                    f"but client version is {client_version}. "
+                    f"Some features may not work as expected."
+                )
+        deserialized = TypeAdapter(target_type).validate_python(
+            _response_json(response)
+        )
         return deserialized
 
 

src/blueapi/service/main.py

@@ -34,6 +34,7 @@
 from starlette.responses import JSONResponse
 from super_state_machine.errors import TransitionError
 
+from blueapi import __version__
 from blueapi.config import ApplicationConfig, OIDCConfig, Tag
 from blueapi.service import interface
 from blueapi.worker import TrackableTask, WorkerState
@@ -123,7 +124,7 @@ def get_app(config: ApplicationConfig):
     app.include_router(secure_router, dependencies=dependencies)
     app.add_exception_handler(KeyError, on_key_error_404)
     app.add_exception_handler(jwt.PyJWTError, on_token_error_401)
-    app.middleware("http")(add_api_version_header)
+    app.middleware("http")(add_version_headers)
     app.middleware("http")(inject_propagated_observability_context)
     app.middleware("http")(log_request_details)
     if config.api.cors:
@@ -573,11 +574,12 @@ def start(config: ApplicationConfig):
     )
 
 
-async def add_api_version_header(
+async def add_version_headers(
     request: Request, call_next: Callable[[Request], Awaitable[Response]]
 ):
     response = await call_next(request)
     response.headers["X-API-Version"] = ApplicationConfig.REST_API_VERSION
+    response.headers["X-BlueAPI-Version"] = __version__
     return response
 
 

src/blueapi/worker/event.py

@@ -113,8 +113,8 @@ class StatusView(BlueapiBaseModel):
         description="Target value operation of progress, if known", default=None
     )
     unit: str = Field(description="Units of progress", default="units")
-    precision: int = Field(
-        description="Sensible precision of progress to display", default=3
+    precision: int | None = Field(
+        description="Sensible precision of progress to display", default=None
     )
     done: bool = Field(
         description="Whether the operation this status describes is complete",

src/blueapi/worker/task_worker.py

@@ -656,7 +656,7 @@ def _on_status_event(
             initial=initial,
             target=target,
             unit=unit or "units",
-            precision=precision or 3,
+            precision=precision,
             done=status.done,
             percentage=percentage,
             time_elapsed=time_elapsed,

tests/system_tests/test_blueapi_system.py

@@ -365,7 +365,7 @@ def test_put_worker_task_fails_if_not_idle(rest_client: BlueapiRestClient):
 
     with pytest.raises(BlueskyRemoteControlError) as exception:
         rest_client.update_worker_task(WorkerTask(task_id=small_task.task_id))
-    assert "Worker already active" in exception.value.args[0]
+    assert exception.value.args[0] == 409
     rest_client.cancel_current_task(WorkerState.ABORTING)
     rest_client.clear_task(small_task.task_id)
     rest_client.clear_task(long_task.task_id)
@@ -378,10 +378,10 @@ def test_get_worker_state(client: BlueapiClient):
 def test_set_state_transition_error(client: BlueapiClient):
     with pytest.raises(BlueskyRemoteControlError) as exception:
         client.resume()
-    assert "Cannot transition from IDLE to RUNNING" in exception.value.args[0]
+    assert "Cannot transition from IDLE to RUNNING" in exception.value.args[1]
     with pytest.raises(BlueskyRemoteControlError) as exception:
         client.pause()
-    assert "Cannot transition from IDLE to PAUSED" in exception.value.args[0]
+    assert "Cannot transition from IDLE to PAUSED" in exception.value.args[1]
 
 
 def test_get_task_by_status(rest_client: BlueapiRestClient):