Include correlation ID in exception message. (bluesky#698)

* Include correlation ID in exception message.

* raise_for_status returns response

* Copyedit comment.

Co-authored-by: Hiran Wijesinghe <[email protected]>

---------

Co-authored-by: Hiran Wijesinghe <[email protected]>

tiled/_tests/test_metrics.py

@@ -1,9 +1,12 @@
 import re
 
+import httpx
+import pytest
 from fastapi import APIRouter
 from starlette.status import HTTP_404_NOT_FOUND, HTTP_500_INTERNAL_SERVER_ERROR
 
 from ..client import Context, from_context
+from ..client.utils import handle_error
 from ..server.app import build_app_from_config
 
 config = {
@@ -49,3 +52,15 @@ def test_error_code():
         response_404 = client.context.http_client.get("/does_not_exist")
         assert response_404.status_code == HTTP_404_NOT_FOUND
         assert total_request_time(client, 404) - baseline_time[404] > 0
+
+
+def test_correlation_id():
+    "Test that exceptions include correlation ID."
+    app = build_app_from_config(config)
+    app.include_router(router)
+    with Context.from_app(app, raise_server_exceptions=False) as context:
+        client = from_context(context)
+        response_500 = client.context.http_client.get("/error")
+        with pytest.raises(httpx.HTTPStatusError) as exc_info:
+            handle_error(response_500)
+        assert "correlation ID" in exc_info.value.args[0]

tiled/client/utils.py

@@ -14,11 +14,54 @@
 MSGPACK_MIME_TYPE = "application/x-msgpack"
 
 
+def raise_for_status(response) -> None:
+    """
+    Raise the `httpx.HTTPStatusError` if one occurred. Include correlation ID.
+    """
+    # This is adapted from the method httpx.Response.raise_for_status, modified to
+    # remove the generic link to HTTP status documentation and include the
+    # correlation ID.
+    request = response._request
+    if request is None:
+        raise RuntimeError(
+            "Cannot call `raise_for_status` as the request "
+            "instance has not been set on this response."
+        )
+
+    if response.is_success:
+        return response
+
+    if response.has_redirect_location:
+        message = (
+            "{error_type} '{0.status_code} {0.reason_phrase}' for url '{0.url}'\n"
+            "Redirect location: '{0.headers[location]}'\n"
+            "For more information, server admin can search server logs for "
+            "correlation ID {0.headers[x-tiled-request-id]}."
+        )
+    else:
+        message = (
+            "{error_type} '{0.status_code} {0.reason_phrase}' for url '{0.url}'\n"
+            "For more information, server admin can search server logs for "
+            "correlation ID {0.headers[x-tiled-request-id]}."
+        )
+
+    status_class = response.status_code // 100
+    error_types = {
+        1: "Informational response",
+        3: "Redirect response",
+        4: "Client error",
+        5: "Server error",
+    }
+    error_type = error_types.get(status_class, "Invalid status code")
+    message = message.format(response, error_type=error_type)
+    raise httpx.HTTPStatusError(message, request=request, response=response)
+
+
 def handle_error(response):
     if not response.is_error:
         return response
     try:
-        response.raise_for_status()
+        raise_for_status(response)
     except httpx.RequestError:
         raise  # Nothing to add in this case; just raise it.
     except httpx.HTTPStatusError as exc: