From 24e1e37ebec33ac7e93496328dcc45ca4a108b34 Mon Sep 17 00:00:00 2001 From: majdyz Date: Fri, 10 Apr 2026 23:52:10 +0000 Subject: [PATCH] fix(util/service): preserve GraphValidationError.node_errors over RPC MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ``GraphValidationError`` carries a structured ``node_errors`` mapping in addition to its top-level message, but RPC serialisation only copied ``exc.args`` — so by the time the exception reached the copilot client (for the schedule path, which goes via ``get_scheduler_client``), ``node_errors`` was ``{}``. That broke the credential-race fallback introduced in the earlier commits in this PR: the helper saw an empty ``node_errors``, concluded it wasn't a credential error, and fell back to the generic ``ErrorResponse`` — exactly the symptom this PR is trying to fix. Fix: - Add an optional ``extras`` dict to ``RemoteCallError`` for exception types that carry structured attributes beyond ``args``. - In ``_handle_internal_http_error``, when the exception is a ``GraphValidationError``, pack ``node_errors`` into ``extras``. - In ``_handle_call_method_response``, when reconstructing a ``GraphValidationError``, read ``extras.node_errors`` and pass it to the constructor. - Add two tests for the round-trip (with and without extras), the latter to ensure backwards compatibility with old server responses. --- .../backend/backend/util/service.py | 30 +++++++++ .../backend/backend/util/service_test.py | 61 +++++++++++++++++++ 2 files changed, 91 insertions(+) diff --git a/autogpt_platform/backend/backend/util/service.py b/autogpt_platform/backend/backend/util/service.py index 459e46f01c..5b1799bf62 100644 --- a/autogpt_platform/backend/backend/util/service.py +++ b/autogpt_platform/backend/backend/util/service.py @@ -159,6 +159,13 @@ class BaseAppService(AppProcess, ABC): class RemoteCallError(BaseModel): type: str = "RemoteCallError" args: Optional[Tuple[Any, ...]] = None + # Optional extras for exception types that carry structured attributes + # beyond ``exc.args``. When set, the client-side handler uses these to + # reconstruct the exception with the original attributes. + # Currently used by ``GraphValidationError.node_errors`` so the + # copilot's credential-race fallback can distinguish credential + # failures from other graph validation errors over RPC. + extras: Optional[dict[str, Any]] = None class UnhealthyServiceError(ValueError): @@ -238,11 +245,21 @@ class AppService(BaseAppService, ABC): f"{request.method} {request.url.path} failed: {exc}", exc_info=exc, ) + extras: Optional[dict[str, Any]] = None + if isinstance(exc, exceptions.GraphValidationError): + # ``exc.args`` only preserves the top-level message; the + # structured ``node_errors`` mapping needs to ride along + # in ``extras`` so the client can rebuild the original + # exception state (used by the copilot credential-race + # fallback to distinguish credential failures from other + # validation errors). + extras = {"node_errors": dict(exc.node_errors)} return responses.JSONResponse( status_code=status_code, content=RemoteCallError( type=str(exc.__class__.__name__), args=exc.args or (str(exc),), + extras=extras, ).model_dump(), ) @@ -614,6 +631,19 @@ def get_service_client( msg = str(args[0]) if args else str(e) raise exception_class({"user_facing_error": {"message": msg}}) + # GraphValidationError carries a structured ``node_errors`` + # attribute that ``exc.args`` alone doesn't preserve. + # If the server included it in ``extras``, thread it + # back into the reconstructed exception. + if issubclass(exception_class, exceptions.GraphValidationError): + msg = str(args[0]) if args else str(e) + node_errors = ( + error_response.extras.get("node_errors") + if error_response.extras + else None + ) + raise exception_class(msg, node_errors=node_errors) + raise exception_class(*args) # Otherwise categorize by HTTP status code diff --git a/autogpt_platform/backend/backend/util/service_test.py b/autogpt_platform/backend/backend/util/service_test.py index e314a47f74..8d5c8444f1 100644 --- a/autogpt_platform/backend/backend/util/service_test.py +++ b/autogpt_platform/backend/backend/util/service_test.py @@ -12,6 +12,7 @@ from prisma.errors import DataError, UniqueViolationError from pydantic import TypeAdapter from backend.data.model import User +from backend.util.exceptions import GraphValidationError from backend.util.service import ( AppService, AppServiceClient, @@ -489,6 +490,66 @@ class TestHTTPErrorRetryBehavior: assert hasattr(exc_info.value, "data") assert isinstance(exc_info.value.data, dict) + def test_graph_validation_error_preserves_node_errors(self): + """GraphValidationError carries a structured ``node_errors`` mapping + in addition to its top-level message. The server-side error handler + packs it into ``RemoteCallError.extras`` and the client-side handler + rebuilds the exception with ``node_errors`` preserved — without this + round-trip the copilot's credential-race fallback can't distinguish + credential failures from other validation errors, and users get a + generic error instead of the inline credentials setup card. + """ + node_errors = { + "some-node-id": { + "credentials": "These credentials are required", + "api_key": "Invalid credentials: not found", + } + } + mock_response = Mock() + mock_response.status_code = 400 + mock_response.json.return_value = { + "type": "GraphValidationError", + "args": ["Graph validation failed: 2 issues on 1 nodes"], + "extras": {"node_errors": node_errors}, + } + mock_response.raise_for_status.side_effect = httpx.HTTPStatusError( + "400 Bad Request", request=Mock(), response=mock_response + ) + + client = get_service_client(ServiceTestClient) + + with pytest.raises(GraphValidationError) as exc_info: + client._handle_call_method_response( # type: ignore[attr-defined] + response=mock_response, method_name="test_method" + ) + + assert "Graph validation failed" in str(exc_info.value) + assert exc_info.value.node_errors == node_errors + + def test_graph_validation_error_without_extras_still_deserializes(self): + """Backwards-compat: old server responses without ``extras`` should + still reconstruct a ``GraphValidationError`` — just with an empty + ``node_errors`` mapping (matches current pre-fix behaviour).""" + mock_response = Mock() + mock_response.status_code = 400 + mock_response.json.return_value = { + "type": "GraphValidationError", + "args": ["Graph validation failed: 1 issues on 1 nodes"], + } + mock_response.raise_for_status.side_effect = httpx.HTTPStatusError( + "400 Bad Request", request=Mock(), response=mock_response + ) + + client = get_service_client(ServiceTestClient) + + with pytest.raises(GraphValidationError) as exc_info: + client._handle_call_method_response( # type: ignore[attr-defined] + response=mock_response, method_name="test_method" + ) + + assert "Graph validation failed" in str(exc_info.value) + assert exc_info.value.node_errors == {} + def test_client_error_status_codes_coverage(self): """Test that various 4xx status codes are all wrapped as HTTPClientError.""" client_error_codes = [400, 401, 403, 404, 405, 409, 422, 429]