feat(mcp): add a single vocabulary for tool failures

Adds lib/errors.py: the exception types the sub-servers raise for a
failure they can describe -- an API rejection, a task that never
finished, bad credentials, a Hub outage -- and render_tool_error, the
one place any of them becomes text a client reads.

Nothing imports it yet. The wrapper that turns these into an MCP error
on every tool, and the servers that raise them, follow in this stack.

mcp_server/prowler_mcp_server/lib/errors.py

@@ -0,0 +1,288 @@
+"""What a Prowler MCP Server failure is, and the sentence it is described with.
+
+MCP draws a line this server used to blur. A tool that *returns* `{"error": ...}`
+produces a successful result (`isError: false`) whose failure is only discoverable by
+guessing which key to look at; a tool that *raises* produces `isError: true`, which
+every client and model already understands as "this call did not work". Moving the
+server onto the second of those needs one vocabulary of failures and one way to render
+them, which is what this module is; raising them is the sub-servers' job.
+
+`render_tool_error` is the single place an exception becomes text a client reads, so
+the same failure can never get described two different ways.
+
+The exception types live here rather than next to the API client because the hub and the
+documentation sub-servers must be able to raise and render them without taking a
+dependency on `prowler_app`.
+"""
+
+from __future__ import annotations
+
+import json
+from dataclasses import dataclass
+from typing import Any
+
+import httpx
+
+# Upstream bodies are not ours and may be large or HTML; enough to diagnose, not enough
+# to flood the model's context.
+_MAX_UPSTREAM_BODY = 500
+
+# The one thing a status code does not say. Appended only when a request that could have
+# changed something did not come back with a verdict, because an agent reads a failure as
+# "nothing happened" and will happily send the write again.
+_UNKNOWN_OUTCOME = (
+    " It may have been carried out anyway, so check the current state before retrying."
+)
+
+
+@dataclass(frozen=True, slots=True)
+class ApiErrorDetail:
+    """One entry of a JSON:API `errors` array.
+
+    The API answers a rejected write with one error *per invalid field*, each naming the
+    field in `source`. Keeping the whole shape is what turns "the request was invalid"
+    into "these two fields were invalid, and here is which".
+
+    Field names are JSON:API's own. `source.pointer` and `source.parameter` are not two
+    spellings of one thing: a pointer is a JSON Pointer into the request *document*
+    (`/data/attributes/provider_id`, as `alerts/errors.py` and `tasks/beat.py` send),
+    while a parameter is a *query parameter* name (`page[size]`, `lookback_days`, as
+    `api/v1/views.py` sends). The spec has a third, `source.header`; the Prowler API
+    never emits one, so there is nothing here to read it into.
+    """
+
+    detail: str | None = None
+    title: str | None = None
+    pointer: str | None = None
+    """JSON:API `source.pointer`: a JSON Pointer into the request document."""
+    parameter: str | None = None
+    """JSON:API `source.parameter`: the query parameter that caused the error."""
+
+    @classmethod
+    def from_jsonapi(cls, error: dict[str, Any]) -> ApiErrorDetail:
+        """Build from a single member of a JSON:API `errors` array.
+
+        `source` is optional and most errors omit it, so it supplies the location only,
+        never whether there is a detail worth reporting.
+        """
+        source = error.get("source", {})
+        return cls(
+            detail=error.get("detail"),
+            title=error.get("title"),
+            pointer=source.get("pointer"),
+            parameter=source.get("parameter"),
+        )
+
+    def render(self) -> str:
+        """The error text, and where the API said it is.
+
+        A pointer is left as-is because a leading `/` already reads as a path into the
+        body. A parameter is labelled, since `(page[size])` on its own would read like
+        one.
+        """
+        text = self.detail or self.title or ""
+        if not text:
+            return ""
+        if self.pointer:
+            return f"{text} ({self.pointer})"
+        if self.parameter:
+            return f"{text} (parameter {self.parameter})"
+        return text
+
+
+def parse_jsonapi_errors(payload: Any) -> tuple[ApiErrorDetail, ...]:
+    """Extract every error from a JSON:API error document.
+
+    Tolerant on purpose: this runs while handling a failure, and a body that is not the
+    document it should be must not turn a useful API error into a parsing traceback.
+    """
+    if not isinstance(payload, dict):
+        return ()
+    errors = payload.get("errors")
+    if not isinstance(errors, list):
+        return ()
+    return tuple(
+        ApiErrorDetail.from_jsonapi(error)
+        for error in errors
+        if isinstance(error, dict)
+    )
+
+
+class ProwlerAPIError(Exception):
+    """An error response returned by the Prowler API.
+
+    Raised only when the API answered with an error status, which tells a caller
+    something no plain exception can: the request reached Prowler and was
+    rejected, so it changed nothing. A timeout or a dropped connection stays a
+    bare exception because the request may well have been processed.
+    """
+
+    def __init__(
+        self,
+        message: str,
+        status_code: int,
+        *,
+        method: str | None = None,
+        path: str | None = None,
+        errors: tuple[ApiErrorDetail, ...] = (),
+    ) -> None:
+        super().__init__(message)
+        self.status_code: int = status_code
+        # Stored as plain strings so this module stays independent of the API client's
+        # HTTPMethod enum; StrEnum members compare equal to their value either way.
+        self.method: str | None = str(method) if method is not None else None
+        self.path: str | None = path
+        self.errors: tuple[ApiErrorDetail, ...] = tuple(errors)
+
+    @property
+    def rejected(self) -> bool:
+        """The request reached Prowler and was refused, so it changed nothing."""
+        return 400 <= self.status_code < 500
+
+
+class ProwlerTaskError(Exception):
+    """A background task this server was waiting on did not complete.
+
+    Separate from `ProwlerAPIError` because the API already accepted the work: the
+    task exists and may still be running, so the outcome is unknown rather than refused.
+    """
+
+    def __init__(self, message: str, *, task_id: str, state: str) -> None:
+        super().__init__(message)
+        self.task_id: str = task_id
+        self.state: str = state
+        """One of `timeout`, `failed` or `cancelled`."""
+
+
+class ProwlerAuthError(ValueError):
+    """The credentials are missing, malformed or expired.
+
+    Subclasses `ValueError` so that the handlers which already treat an
+    authentication failure as a refusal-before-send keep working. It is matched by name
+    in `render_tool_error` rather than by that base class, so it is described as the
+    credential problem it is instead of falling through to the bug branch.
+    """
+
+
+class ProwlerHubError(Exception):
+    """The Prowler Hub answered with an error status.
+
+    The Hub is a separate public service with its own client, so its failures cannot be
+    `ProwlerAPIError`. Everything the Hub exposes is a read.
+    """
+
+    def __init__(
+        self,
+        message: str,
+        *,
+        status_code: int,
+        path: str,
+        body: str | None = None,
+    ) -> None:
+        super().__init__(message)
+        self.status_code: int = status_code
+        self.path: str = path
+        self.body: str | None = body
+
+
+def _upstream_detail(body: str | None) -> str:
+    """A readable line from an error body that is not JSON:API.
+
+    The Prowler API answers with a JSON:API document, which is parsed into
+    `ApiErrorDetail`. Every other host this server talks to has its own shape: the Hub
+    answers `{"error": "Not found"}`, GitHub answers plain text, a proxy in between may
+    answer HTML. Relaying any of those verbatim puts braces and markup in front of the
+    model, so the message is pulled out when there is one and truncated when there is
+    not.
+    """
+    if not body:
+        return ""
+    text = body.strip()
+    try:
+        parsed = json.loads(text)
+    except ValueError:
+        parsed = None
+    if isinstance(parsed, dict):
+        for key in ("error", "message", "detail"):
+            value = parsed.get(key)
+            if isinstance(value, str) and value:
+                return value
+    return f"{text[:_MAX_UPSTREAM_BODY]}..." if len(text) > _MAX_UPSTREAM_BODY else text
+
+
+def render_tool_error(error: Exception, *, warn: bool = True) -> str:
+    """Describe an exception in the plainest sentence that keeps every useful detail.
+
+    Callers that surface a failure from anywhere other than a raised exception -- a
+    message they write themselves, a field of a structured result -- go through here
+    too, which is what keeps one failure from being described two different ways.
+
+    Pass `warn=False` when the caller already states the outcome, which a structured
+    result reporting `status="unknown"` does by definition. Otherwise the generic
+    warning lands next to a more specific one saying the same thing.
+
+    Ordered most specific first: `ProwlerAuthError` is a `ValueError` and
+    `httpx.TimeoutException` is a `RequestError`, so the general branches come last.
+    """
+    unknown = _UNKNOWN_OUTCOME if warn else ""
+
+    if isinstance(error, ProwlerAPIError):
+        operation = (
+            " ".join(p for p in (error.method, error.path) if p) or "The request"
+        )
+        details = "; ".join(text for text in (d.render() for d in error.errors) if text)
+        message = f"{operation} failed with HTTP {error.status_code}."
+        if details:
+            message = f"{message} {details}"
+        # A 4xx is a refusal, so it changed nothing and needs no warning.
+        if error.rejected or error.method == "GET":
+            return message
+        return message + unknown
+
+    if isinstance(error, ProwlerTaskError):
+        # The API accepted the work before the wait failed, so the outcome is open
+        # whichever way the task ended.
+        return f"{error}{unknown}"
+
+    if isinstance(error, ProwlerAuthError):
+        return f"Prowler authentication failed: {error}"
+
+    if isinstance(error, ProwlerHubError):
+        # Same shape as the API branch, with the service named because the Hub can be
+        # down while the API is fine. Everything the Hub exposes is a GET.
+        message = f"Prowler Hub GET {error.path} failed with HTTP {error.status_code}."
+        detail = _upstream_detail(error.body)
+        return f"{message} {detail}" if detail else message
+
+    if isinstance(error, httpx.HTTPStatusError):
+        # An upstream that is not the Prowler API, such as the external-URL fetch.
+        request = error.request
+        message = (
+            f"{request.method} {request.url} failed with HTTP "
+            f"{error.response.status_code}."
+        )
+        detail = _upstream_detail(error.response.text)
+        return f"{message} {detail}" if detail else message
+
+    if isinstance(error, httpx.RequestError):
+        # No answer at all: a timeout, a dropped connection, a DNS failure.
+        try:
+            operation = f"{error.request.method} {error.request.url}"
+            method = error.request.method
+        except RuntimeError:
+            # httpx only attaches the request once it has one, and reading it before
+            # then raises. Never let that hide the failure being reported.
+            operation, method = "The request", None
+        suffix = "" if method == "GET" else unknown
+        return f"{operation} got no answer ({type(error).__name__}: {error}).{suffix}"
+
+    # No `ValueError` branch, deliberately. A message written for the caller is raised
+    # as a `ToolError`, which never reaches here. What is left -- a model factory
+    # rejecting an API payload, a pydantic `ValidationError`, an `int()` on something
+    # that is not a number -- is this server or the API breaking its own contract, and
+    # saying so is the only useful thing to tell a caller who cannot fix it.
+    return (
+        f"The Prowler MCP Server hit an unexpected {type(error).__name__}: {error}. "
+        "This is a bug in the server, not something you can fix by changing the "
+        "arguments."
+    )

mcp_server/tests/lib/test_errors.py

@@ -0,0 +1,311 @@
+"""Tests for the sentence a failure is described with.
+
+These assert on the *text* a model reads, because that text is the whole contract: a
+`ToolError` carries nothing else. What matters is that the API's own words survive
+intact, and that a write whose outcome nobody can report says so.
+
+How that sentence reaches a client -- and that no tool can escape it -- is
+`test_server.py`.
+"""
+
+import httpx
+import pytest
+
+from prowler_mcp_server.lib.errors import (
+    ApiErrorDetail,
+    ProwlerAPIError,
+    ProwlerAuthError,
+    ProwlerHubError,
+    ProwlerTaskError,
+    parse_jsonapi_errors,
+    render_tool_error,
+)
+
+MAY_HAVE_LANDED = "It may have been carried out anyway"
+
+
+# --------------------------------------------------------------------------- parsing
+
+
+def test_every_error_of_the_document_is_preserved():
+    """A rejected write names one error per invalid field; all of them matter."""
+    errors = parse_jsonapi_errors(
+        {
+            "errors": [
+                {"status": "400", "detail": "This field may not be blank."},
+                {"status": "400", "detail": "Enter a valid URL."},
+            ]
+        }
+    )
+
+    assert [error.detail for error in errors] == [
+        "This field may not be blank.",
+        "Enter a valid URL.",
+    ]
+
+
+def test_a_query_parameter_error_is_not_dressed_up_as_a_body_path():
+    """`source.parameter` and `source.pointer` are different places, per JSON:API.
+
+    The API sends a parameter for a bad query string (`api/v1/views.py` answers
+    `page[size]` and `lookback_days` that way) and a pointer for a bad body field.
+    Rendering a parameter bare would read as though `page[size]` were a path into the
+    document, which is somewhere the caller never put it.
+    """
+    (error,) = parse_jsonapi_errors(
+        {
+            "errors": [
+                {
+                    "detail": "invalid parameter 'page[size]'",
+                    "source": {"parameter": "page[size]"},
+                }
+            ]
+        }
+    )
+
+    assert error.parameter == "page[size]"
+    assert error.pointer is None
+    assert error.render() == "invalid parameter 'page[size]' (parameter page[size])"
+
+
+def test_the_field_an_error_points_at_is_kept():
+    """`source.pointer` is what turns "invalid" into "this field is invalid"."""
+    (error,) = parse_jsonapi_errors(
+        {
+            "errors": [
+                {
+                    "detail": "This field may not be blank.",
+                    "source": {"pointer": "/data/attributes/name"},
+                }
+            ]
+        }
+    )
+
+    assert error.render() == ("This field may not be blank. (/data/attributes/name)")
+
+
+def test_an_error_with_only_a_title_still_says_something():
+    """`detail` is the useful field, but the API does not always send one."""
+    assert ApiErrorDetail(title="Not Found").render() == "Not Found"
+
+
+@pytest.mark.parametrize(
+    "payload",
+    [None, "not a document", {}, {"errors": "not a list"}, {"errors": [None]}],
+    ids=["none", "text", "empty", "errors-not-a-list", "member-not-a-dict"],
+)
+def test_a_body_that_is_not_an_error_document_is_tolerated(payload):
+    """Parsing runs while handling a failure; it must not become the failure."""
+    assert parse_jsonapi_errors(payload) == ()
+
+
+def test_an_error_with_nothing_in_it_renders_empty():
+    """Rendered to nothing rather than to punctuation, so composition can skip it."""
+    assert ApiErrorDetail().render() == ""
+
+
+# --------------------------------------------------------------------- api failures
+
+
+def test_a_failed_read_names_the_call_and_the_reason():
+    message = render_tool_error(
+        ProwlerAPIError(
+            "API request failed: 404 - Not found.",
+            404,
+            method="GET",
+            path="/findings/nope",
+            errors=parse_jsonapi_errors(
+                {"errors": [{"detail": "No Finding matches the given query."}]}
+            ),
+        )
+    )
+
+    assert message == (
+        "GET /findings/nope failed with HTTP 404. No Finding matches the given query."
+    )
+
+
+def test_every_error_of_a_rejected_write_reaches_the_client():
+    """The API rejects a write with one error per invalid field, and all of them help."""
+    message = render_tool_error(
+        ProwlerAPIError(
+            "API request failed: 400 - blank",
+            400,
+            method="POST",
+            path="/integrations",
+            errors=parse_jsonapi_errors(
+                {
+                    "errors": [
+                        {
+                            "detail": "This field may not be blank.",
+                            "source": {"pointer": "/data/attributes/bucket_name"},
+                        },
+                        {"detail": "Enter a valid URL."},
+                    ]
+                }
+            ),
+        )
+    )
+
+    assert message == (
+        "POST /integrations failed with HTTP 400. "
+        "This field may not be blank. (/data/attributes/bucket_name); "
+        "Enter a valid URL."
+    )
+
+
+def test_a_rejected_write_gets_no_warning():
+    """A 4xx changed nothing, so there is nothing to warn about."""
+    message = render_tool_error(
+        ProwlerAPIError("boom", 400, method="POST", path="/scans")
+    )
+
+    assert MAY_HAVE_LANDED not in message
+
+
+def test_a_write_that_hit_a_server_error_warns_it_may_have_landed():
+    """The API validates and queues before answering, so a 500 may have gone through."""
+    message = render_tool_error(
+        ProwlerAPIError("boom", 500, method="DELETE", path="/integrations/i1")
+    )
+
+    assert message.startswith("DELETE /integrations/i1 failed with HTTP 500.")
+    assert MAY_HAVE_LANDED in message
+
+
+def test_a_failed_read_never_warns():
+    """A read cannot have changed anything, whatever went wrong."""
+    message = render_tool_error(
+        ProwlerAPIError("boom", 500, method="GET", path="/scans")
+    )
+
+    assert MAY_HAVE_LANDED not in message
+
+
+# ------------------------------------------------------------------ no answer at all
+
+
+def test_a_write_that_got_no_answer_warns_it_may_have_landed():
+    """A timeout is the case the warning exists for."""
+    request = httpx.Request("POST", "https://api.testing.invalid/api/v1/scans")
+    message = render_tool_error(httpx.ReadTimeout("timed out", request=request))
+
+    assert "POST https://api.testing.invalid/api/v1/scans got no answer" in message
+    assert "ReadTimeout" in message
+    assert MAY_HAVE_LANDED in message
+
+
+def test_a_read_that_got_no_answer_does_not_warn():
+    request = httpx.Request("GET", "https://api.testing.invalid/api/v1/findings")
+    message = render_tool_error(httpx.ReadTimeout("timed out", request=request))
+
+    assert MAY_HAVE_LANDED not in message
+
+
+def test_a_dropped_connection_names_what_went_wrong():
+    request = httpx.Request("POST", "https://api.testing.invalid/api/v1/scans")
+    message = render_tool_error(httpx.ConnectError("connection reset", request=request))
+
+    assert "ConnectError: connection reset" in message
+
+
+def test_an_unfinished_task_warns_it_may_have_landed():
+    """The API already accepted the work, so the outcome is open, not refused."""
+    message = render_tool_error(
+        ProwlerTaskError(
+            "Task t1 polling timed out after 60 seconds.", task_id="t1", state="timeout"
+        )
+    )
+
+    assert message.startswith("Task t1 polling timed out after 60 seconds.")
+    assert MAY_HAVE_LANDED in message
+
+
+# -------------------------------------------------------------- refusals before send
+
+
+def test_a_stray_value_error_is_reported_as_a_bug():
+    """The distinction the previous passthrough branch could not make.
+
+    A model factory rejecting an API payload, or an `int()` on something that is not a
+    number, is not the caller's mistake. Describing it like a validation message sends
+    an agent off rewriting arguments that were never the problem.
+    """
+    message = render_tool_error(
+        ValueError("Missing pagination metadata in API response")
+    )
+
+    assert "unexpected ValueError" in message
+    assert "bug in the server" in message
+
+
+def test_an_authentication_failure_says_so():
+    """A `ValueError` subclass, so it must be recognised before the generic branch."""
+    assert render_tool_error(ProwlerAuthError("Token has expired")) == (
+        "Prowler authentication failed: Token has expired"
+    )
+
+
+# ---------------------------------------------------------------------- other hosts
+
+
+def test_a_hub_failure_reads_like_an_api_failure():
+    """Same sentence as the Prowler API, with the service named.
+
+    The Hub can be down while the API is fine, so which one failed is worth the two
+    extra words -- but the shape must not differ, or the two look like two contracts.
+    """
+    message = render_tool_error(
+        ProwlerHubError(
+            "hub failed",
+            status_code=404,
+            path="/check/test",
+            body='{"error": "Not found"}',
+        )
+    )
+
+    assert message == "Prowler Hub GET /check/test failed with HTTP 404. Not found"
+
+
+@pytest.mark.parametrize(
+    ("body", "expected"),
+    [
+        ('{"error": "Not found"}', "Not found"),
+        ('{"message": "Bad gateway"}', "Bad gateway"),
+        ('{"detail": "Rate limited"}', "Rate limited"),
+        ("Service Unavailable", "Service Unavailable"),
+        ('{"unexpected": "shape"}', '{"unexpected": "shape"}'),
+    ],
+    ids=["error", "message", "detail", "plain-text", "unknown-json"],
+)
+def test_an_upstream_message_is_pulled_out_of_whatever_shape_it_came_in(body, expected):
+    """Hosts that are not the Prowler API each have their own error shape.
+
+    Relaying the raw body puts JSON braces, or a whole HTML page, in front of the model.
+    """
+    message = render_tool_error(
+        ProwlerHubError("hub failed", status_code=500, path="/checks", body=body)
+    )
+
+    assert message.endswith(expected)
+
+
+def test_an_upstream_body_is_truncated():
+    """An HTML error page must not flood the model's context."""
+    message = render_tool_error(
+        ProwlerHubError("hub failed", status_code=500, path="/checks", body="x" * 2000)
+    )
+
+    assert "x" * 500 in message
+    assert "x" * 501 not in message
+
+
+# --------------------------------------------------------------------- server bugs
+
+
+def test_a_bug_in_this_server_is_reported_as_a_bug():
+    """Named as ours, so the caller stops trying to fix it by changing arguments."""
+    message = render_tool_error(KeyError("attributes"))
+
+    assert "unexpected KeyError" in message
+    assert "bug in the server" in message