diff --git a/packages/cli/src/pipefy_cli/commands/_common.py b/packages/cli/src/pipefy_cli/commands/_common.py index 11e883fa..f12e806f 100644 --- a/packages/cli/src/pipefy_cli/commands/_common.py +++ b/packages/cli/src/pipefy_cli/commands/_common.py @@ -10,8 +10,13 @@ from typing import Any, TypeVar import typer -from gql.transport.exceptions import TransportError, TransportQueryError -from pipefy_sdk import PipefyClient, PipefySettings, stream_bytes +from gql.transport.exceptions import TransportError +from pipefy_sdk import ( + PipefyClient, + PipefyGraphQLError, + PipefySettings, + stream_bytes, +) from pipefy_sdk.exceptions import PipefyError from pipefy_sdk.label_color import normalize_label_color from pipefy_sdk.report_filter_preflight import prepare_report_cards_filter @@ -221,7 +226,7 @@ async def _run() -> _T: raise typer.Exit(value_error_exit_code) from exc except BrokenPipeError: raise typer.Exit(0) from None - except TransportQueryError as exc: + except PipefyGraphQLError as exc: typer.echo(_format_transport_query_error(exc), err=True) raise typer.Exit(1) from exc except TransportError as exc: @@ -229,7 +234,7 @@ async def _run() -> _T: raise typer.Exit(1) from exc -def _format_transport_query_error(exc: TransportQueryError) -> str: +def _format_transport_query_error(exc: PipefyGraphQLError) -> str: """Render a GraphQL transport error as a clean single-line message for the CLI. Falls back to ``str(exc)`` when the structured ``errors`` payload is missing or empty. @@ -243,7 +248,7 @@ def _format_transport_query_error(exc: TransportQueryError) -> str: return f"{message} ({code})" if code else message -def format_card_get_transport_query_error(exc: TransportQueryError) -> str: +def format_card_get_transport_query_error(exc: PipefyGraphQLError) -> str: """Like :func:`_format_transport_query_error` with a hint for missing/deleted cards.""" base = _format_transport_query_error(exc) errors = getattr(exc, "errors", None) or [] @@ -334,7 +339,7 @@ def run_cli_command( coro_factory: Callable[[PipefyClient], Awaitable[_R]], *, exit_code_2_on_value_error: bool = True, - format_transport_query_error: Callable[[TransportQueryError], str] | None = None, + format_transport_query_error: Callable[[PipefyGraphQLError], str] | None = None, ) -> None: """Run an async coroutine factory with a configured client and render the result. @@ -358,7 +363,7 @@ async def _run() -> _R: except PipefyError as exc: typer.echo(str(exc), err=True) raise typer.Exit(1) from exc - except TransportQueryError as exc: + except PipefyGraphQLError as exc: typer.echo(transport_fmt(exc), err=True) raise typer.Exit(1) from exc except TransportError as exc: diff --git a/packages/cli/src/pipefy_cli/commands/auth.py b/packages/cli/src/pipefy_cli/commands/auth.py index 8dccf341..7f8ac04e 100644 --- a/packages/cli/src/pipefy_cli/commands/auth.py +++ b/packages/cli/src/pipefy_cli/commands/auth.py @@ -12,7 +12,6 @@ import typer from gql.transport.exceptions import ( TransportError, - TransportQueryError, TransportServerError, ) from pipefy_auth import ( @@ -33,7 +32,7 @@ store_session, ) from pipefy_auth.settings import _LEGACY_ENV_KEYS_TO_NEW -from pipefy_sdk import MePayload, PipefySettings +from pipefy_sdk import MePayload, PipefyGraphQLError, PipefySettings from pipefy_cli._docs import DOCS_CLI_AUTH_REF from pipefy_cli.auth import ( @@ -402,7 +401,7 @@ def _fetch_identity( raise _StatusExit( report=report, exit_code=1, stderr=f"Identity fetch failed: {exc}" ) from exc - except (TransportQueryError, TransportError) as exc: + except (PipefyGraphQLError, TransportError) as exc: raise _StatusExit( report=report, exit_code=1, stderr=f"Pipefy transport error: {exc}" ) from exc diff --git a/packages/cli/tests/test_card_get.py b/packages/cli/tests/test_card_get.py index 56bc5444..fe03c493 100644 --- a/packages/cli/tests/test_card_get.py +++ b/packages/cli/tests/test_card_get.py @@ -8,8 +8,7 @@ import pytest from _shared.live_settings import live_pipefy_settings, require_live_creds -from gql.transport.exceptions import TransportQueryError -from pipefy_sdk import PipefySettings +from pipefy_sdk import PipefyGraphQLError, PipefySettings from pipefy_sdk.exceptions import PipefyAPIError from pipefy_cli.main import app @@ -80,9 +79,8 @@ def test_card_get_permission_denied_hint_on_stderr( monkeypatch.setenv("PIPEFY_SERVICE_ACCOUNT_CLIENT_ID", "cid") monkeypatch.setenv("PIPEFY_SERVICE_ACCOUNT_CLIENT_SECRET", "sec") - exc = TransportQueryError( - "unused", - errors=[{"message": "Forbidden", "extensions": {"code": "PERMISSION_DENIED"}}], + exc = PipefyGraphQLError( + [{"message": "Forbidden", "extensions": {"code": "PERMISSION_DENIED"}}] ) mock_client = MagicMock() mock_client.get_card = AsyncMock(side_effect=exc) diff --git a/packages/mcp/src/pipefy_mcp/tools/ai_tool_helpers.py b/packages/mcp/src/pipefy_mcp/tools/ai_tool_helpers.py index a4d12602..38d2fe9a 100644 --- a/packages/mcp/src/pipefy_mcp/tools/ai_tool_helpers.py +++ b/packages/mcp/src/pipefy_mcp/tools/ai_tool_helpers.py @@ -21,7 +21,6 @@ from pipefy_mcp.tools.graphql_error_helpers import ( extract_error_strings, - strip_internal_api_diagnostic_markers, ) from pipefy_mcp.tools.tool_error_envelope import ( ToolErrorDetail, @@ -306,9 +305,8 @@ def enrich_behavior_error( ) -> str: """Build an enriched error message with behavior context and actionable hints. - Extracts GraphQL messages, strips internal_api-style ``[code=…]`` / - ``[correlation_id=…]`` markers from the primary line, appends a behavior - summary, and matches known error patterns to actionable advice. + Extracts the GraphQL messages, appends a behavior summary, and matches known + error patterns to actionable advice. Args: exc: The exception from the service call. @@ -316,7 +314,7 @@ def enrich_behavior_error( """ msgs = extract_error_strings(exc) base = "; ".join(msgs) if msgs else str(exc) - base = strip_internal_api_diagnostic_markers(base).strip() + base = base.strip() if not base: base = _BEHAVIOR_ERROR_EMPTY_AFTER_SANITIZE diff --git a/packages/mcp/src/pipefy_mcp/tools/automation_tool_helpers.py b/packages/mcp/src/pipefy_mcp/tools/automation_tool_helpers.py index 84eef899..4cab6641 100644 --- a/packages/mcp/src/pipefy_mcp/tools/automation_tool_helpers.py +++ b/packages/mcp/src/pipefy_mcp/tools/automation_tool_helpers.py @@ -17,7 +17,6 @@ extract_error_strings, extract_graphql_correlation_id, extract_graphql_error_codes, - strip_internal_api_diagnostic_markers, try_enrich_graphql_error, with_debug_suffix, ) @@ -173,7 +172,6 @@ async def handle_automation_tool_graphql_error( msgs = extract_error_strings(exc) base = "; ".join(msgs) if msgs else _AUTOMATION_REQUEST_FAILED - base = strip_internal_api_diagnostic_markers(base) if not base.strip(): base = _AUTOMATION_REQUEST_FAILED base = with_debug_suffix(base, debug=debug, codes=codes, correlation_id=cid) diff --git a/packages/mcp/src/pipefy_mcp/tools/graphql_error_helpers.py b/packages/mcp/src/pipefy_mcp/tools/graphql_error_helpers.py index c8ef3b93..a8ca00de 100644 --- a/packages/mcp/src/pipefy_mcp/tools/graphql_error_helpers.py +++ b/packages/mcp/src/pipefy_mcp/tools/graphql_error_helpers.py @@ -37,15 +37,6 @@ import pipefy_mcp.settings as _settings_mod from pipefy_mcp.tools.tool_error_envelope import tool_error -# Suffixes appended by the Internal API executor for service-layer diagnostics; -# MCP tools strip these from default user-visible errors. -_INTERNAL_API_CODE_SUFFIX_RE = re.compile(r"\s*\[code=[^\]]*\]") -_INTERNAL_API_CORRELATION_SUFFIX_RE = re.compile(r"\s*\[correlation_id=[^\]]*\]") -_INTERNAL_API_CODE_BRACKET_CAPTURE_RE = re.compile(r"\[code=([^\]]*)\]") -_INTERNAL_API_CORRELATION_BRACKET_CAPTURE_RE = re.compile( - r"\[correlation_id=([^\]]*)\]" -) - _DICT_REPR_PREFIX_RE = re.compile(r"^\s*\{\s*['\"]message['\"]\s*:") @@ -73,54 +64,11 @@ def _try_extract_message_from_dict_repr(raw: str) -> str | None: return None -def strip_internal_api_diagnostic_markers(message: str) -> str: - """Remove ``[code=…]`` / ``[correlation_id=…]`` markers from a message string. - - The Internal API executor appends these to GraphQL error text for logs and - tests. Multiple occurrences (e.g. ``; ``-joined errors) are all removed. - - Args: - message: Raw error text, often ``str(ValueError(...))`` from the executor. - """ - stripped = _INTERNAL_API_CORRELATION_SUFFIX_RE.sub("", message) - stripped = _INTERNAL_API_CODE_SUFFIX_RE.sub("", stripped) - return stripped.strip() - - -def extract_internal_api_bracket_codes(message: str) -> list[str]: - """Collect distinct ``code`` values from ``[code=…]`` markers (Internal API executor). - - Args: - message: Raw exception text before stripping markers. - """ - seen: set[str] = set() - out: list[str] = [] - for raw_code in _INTERNAL_API_CODE_BRACKET_CAPTURE_RE.findall(message): - code = raw_code.strip() - if code and code not in seen: - seen.add(code) - out.append(code) - return out - - -def extract_internal_api_bracket_correlation_id(message: str) -> str | None: - """Return the first non-empty correlation id from ``[correlation_id=…]`` markers. - - Args: - message: Raw exception text before stripping markers. - """ - for raw_cid in _INTERNAL_API_CORRELATION_BRACKET_CAPTURE_RE.findall(message): - cid = raw_cid.strip() - if cid: - return cid - return None - - def extract_error_strings(exc: BaseException) -> list[str]: """Best-effort extraction of error messages from gql/GraphQL exceptions. - When the exception carries a structured ``errors`` list (e.g. gql - ``TransportQueryError``), only the extracted ``message`` strings are + When the exception carries a structured ``errors`` list (e.g. + ``PipefyGraphQLError``), only the extracted ``message`` strings are returned; the raw ``str(exc)`` is skipped because it often contains the full error dict with ``locations`` / ``extensions`` noise. The raw string is used as a fallback only when no structured messages @@ -173,7 +121,6 @@ def extract_graphql_error_codes(exc: BaseException) -> list[str]: if raw: for match in re.findall(r"""['"]code['"]\s*[:=]\s*['"]([A-Z_]+)['"]""", raw): codes.append(match) - codes.extend(extract_internal_api_bracket_codes(raw)) seen: set[str] = set() unique: list[str] = [] @@ -185,7 +132,23 @@ def extract_graphql_error_codes(exc: BaseException) -> list[str]: def extract_graphql_correlation_id(exc: BaseException) -> str | None: - """Best-effort extraction of correlation_id from GraphQL exception strings.""" + """Best-effort extraction of correlation_id from a GraphQL exception. + + Reads the structured ``errors`` list first (each error's ``extensions``), + then falls back to a regex over ``str(exc)``. + """ + errors = getattr(exc, "errors", None) + if isinstance(errors, list): + for item in errors: + if not isinstance(item, dict): + continue + extensions = item.get("extensions") + if not isinstance(extensions, dict): + continue + correlation_id = extensions.get("correlation_id") + if isinstance(correlation_id, str) and correlation_id: + return correlation_id + raw = str(exc) if not raw: return None @@ -193,7 +156,7 @@ def extract_graphql_correlation_id(exc: BaseException) -> str | None: match = re.search(r"""['"]correlation_id['"]\s*[:=]\s*['"]([^'"]+)['"]""", raw) if match: return match.group(1) - return extract_internal_api_bracket_correlation_id(raw) + return None def with_debug_suffix( @@ -581,10 +544,7 @@ async def enrich_permission_denied_error( "extract_error_strings", "extract_graphql_correlation_id", "extract_graphql_error_codes", - "extract_internal_api_bracket_codes", - "extract_internal_api_bracket_correlation_id", "handle_tool_graphql_error", - "strip_internal_api_diagnostic_markers", "try_enrich_graphql_error", "with_debug_suffix", ] diff --git a/packages/mcp/src/pipefy_mcp/tools/observability_tools.py b/packages/mcp/src/pipefy_mcp/tools/observability_tools.py index 9457de8d..3f989e97 100644 --- a/packages/mcp/src/pipefy_mcp/tools/observability_tools.py +++ b/packages/mcp/src/pipefy_mcp/tools/observability_tools.py @@ -52,8 +52,8 @@ def _rewrite_ai_agent_log_not_found(exc: BaseException, log_uuid: str) -> str | The ``aiAgentLogDetails`` resolver looks up by ``AutomationAction`` under the hood; when the UUID isn't found, the upstream error string exposes that internal type (``"Couldn't find AutomationAction with 'id'=..."``). - ``TransportQueryError`` hides per-error messages behind ``.errors``, so we - check both ``str(exc)`` and the structured error list. + ``PipefyGraphQLError`` keeps per-error messages in ``.errors``, so we check + both ``str(exc)`` and the structured error list. """ candidates = [str(exc), *extract_error_strings(exc)] for msg in candidates: diff --git a/packages/mcp/src/pipefy_mcp/tools/pipe_config_tools.py b/packages/mcp/src/pipefy_mcp/tools/pipe_config_tools.py index 03a5a972..eaafffa5 100644 --- a/packages/mcp/src/pipefy_mcp/tools/pipe_config_tools.py +++ b/packages/mcp/src/pipefy_mcp/tools/pipe_config_tools.py @@ -73,9 +73,9 @@ async def _diagnose_phase_field_cascade( """ if pipe_id is None: return None - # gql's ``TransportQueryError`` hides the structured per-error ``message`` - # and ``extensions.code`` behind attributes, so ``str(exc)`` alone only - # returns the outer wrapper ("GraphQL Error"). Combine both signals. + # ``PipefyGraphQLError`` keeps the structured per-error ``message`` and + # ``extensions.code`` in ``.errors``, so ``str(exc)`` alone only returns the + # joined message text. Combine both signals. signal_parts: list[str] = [str(exc).lower()] signal_parts.extend(m.lower() for m in extract_error_strings(exc)) signal_parts.extend(c.lower() for c in extract_graphql_error_codes(exc)) diff --git a/packages/mcp/src/pipefy_mcp/tools/portal_tool_helpers.py b/packages/mcp/src/pipefy_mcp/tools/portal_tool_helpers.py index 273d6684..e3602db3 100644 --- a/packages/mcp/src/pipefy_mcp/tools/portal_tool_helpers.py +++ b/packages/mcp/src/pipefy_mcp/tools/portal_tool_helpers.py @@ -11,14 +11,13 @@ from collections.abc import Awaitable, Callable from typing import Any -from gql.transport.exceptions import TransportQueryError +from pipefy_sdk import PipefyGraphQLError from pipefy_sdk.exceptions import PortalPermissionError from pydantic import ValidationError from pipefy_mcp.tools.graphql_error_helpers import ( extract_error_strings, extract_graphql_error_codes, - strip_internal_api_diagnostic_markers, ) from pipefy_mcp.tools.introspection_tool_helpers import ( build_error_payload, @@ -63,16 +62,12 @@ def map_portal_error_to_message(exc: BaseException) -> str: if "PERMISSION_DENIED" in codes or "permission denied" in lowered: return _PORTAL_PERMISSION_GUIDANCE - if isinstance(exc, TransportQueryError): + if isinstance(exc, PipefyGraphQLError): messages = extract_error_strings(exc) if messages: - return strip_internal_api_diagnostic_markers("; ".join(messages)) + return "; ".join(messages) - return ( - strip_internal_api_diagnostic_markers(text) - if text - else "Portal operation failed. Try again or contact support." - ) + return text or "Portal operation failed. Try again or contact support." def validate_tool_ids( diff --git a/packages/mcp/tests/tools/test_ai_automation_tools.py b/packages/mcp/tests/tools/test_ai_automation_tools.py index 46e2bff8..b7c8a931 100644 --- a/packages/mcp/tests/tools/test_ai_automation_tools.py +++ b/packages/mcp/tests/tools/test_ai_automation_tools.py @@ -8,7 +8,7 @@ from mcp.shared.memory import ( create_connected_server_and_client_session as create_client_session, ) -from pipefy_sdk import PipefyClient +from pipefy_sdk import PipefyClient, PipefyGraphQLError from pipefy_mcp.tools.ai_automation_tools import AiAutomationTools from pipefy_mcp.tools.tool_error_envelope import tool_error_message @@ -596,8 +596,16 @@ async def test_internal_api_style_error_strips_code_and_correlation_from_payload mock_pipefy_client, extract_payload, ): - mock_pipefy_client.create_ai_automation.side_effect = ValueError( - "Invalid prompt [code=INVALID_PROMPT] [correlation_id=abc-123-def]" + mock_pipefy_client.create_ai_automation.side_effect = PipefyGraphQLError( + [ + { + "message": "Invalid prompt", + "extensions": { + "code": "INVALID_PROMPT", + "correlation_id": "abc-123-def", + }, + } + ] ) async with client_session as session: result = await session.call_tool( @@ -625,8 +633,16 @@ async def test_create_debug_true_includes_correlation_and_codes_in_error( mock_pipefy_client, extract_payload, ): - mock_pipefy_client.create_ai_automation.side_effect = ValueError( - "Invalid prompt [code=INVALID_PROMPT] [correlation_id=abc-123-def]" + mock_pipefy_client.create_ai_automation.side_effect = PipefyGraphQLError( + [ + { + "message": "Invalid prompt", + "extensions": { + "code": "INVALID_PROMPT", + "correlation_id": "abc-123-def", + }, + } + ] ) async with client_session as session: result = await session.call_tool( @@ -788,14 +804,19 @@ async def test_service_error_returns_error_payload( assert payload["success"] is False assert "error" in payload - async def test_internal_api_style_error_strips_code_and_correlation_on_update( + async def test_update_structured_error_omits_markers_from_message( self, client_session, mock_pipefy_client, extract_payload, ): - mock_pipefy_client.update_ai_automation.side_effect = ValueError( - "Not found [code=NOT_FOUND] [correlation_id=corr-9]" + mock_pipefy_client.update_ai_automation.side_effect = PipefyGraphQLError( + [ + { + "message": "Not found", + "extensions": {"code": "NOT_FOUND", "correlation_id": "corr-9"}, + } + ] ) async with client_session as session: result = await session.call_tool( @@ -818,8 +839,16 @@ async def test_update_debug_true_includes_correlation_and_codes( mock_pipefy_client, extract_payload, ): - mock_pipefy_client.update_ai_automation.side_effect = ValueError( - "Not found [code=NOT_FOUND] [correlation_id=corr-9]" + mock_pipefy_client.update_ai_automation.side_effect = PipefyGraphQLError( + [ + { + "message": "Not found", + "extensions": { + "code": "NOT_FOUND", + "correlation_id": "corr-9", + }, + } + ] ) async with client_session as session: result = await session.call_tool( @@ -869,8 +898,16 @@ async def test_update_permission_denied_gets_gap_a_ambiguity_hint( mock_pipefy_client, extract_payload, ): - mock_pipefy_client.update_ai_automation.side_effect = ValueError( - "Permission denied [code=PERMISSION_DENIED] [correlation_id=c1]" + mock_pipefy_client.update_ai_automation.side_effect = PipefyGraphQLError( + [ + { + "message": "Permission denied", + "extensions": { + "code": "PERMISSION_DENIED", + "correlation_id": "c1", + }, + } + ] ) async with client_session as session: result = await session.call_tool( diff --git a/packages/mcp/tests/tools/test_ai_tool_helpers.py b/packages/mcp/tests/tools/test_ai_tool_helpers.py index ea8410a3..1344c4fb 100644 --- a/packages/mcp/tests/tools/test_ai_tool_helpers.py +++ b/packages/mcp/tests/tools/test_ai_tool_helpers.py @@ -3,6 +3,7 @@ import pytest from _shared.fixture_ids import make_pipe_id from gql.transport.exceptions import TransportQueryError +from pipefy_sdk import PipefyGraphQLError from pipefy_mcp.tools.ai_tool_helpers import ( build_create_agent_success, @@ -196,9 +197,19 @@ def test_enrich_with_empty_behaviors(): @pytest.mark.unit -def test_enrich_strips_internal_api_diagnostic_markers(): - exc = ValueError( - "Invalid prompt [code=INVALID_PROMPT] [correlation_id=secret-correlation-uuid]" +def test_enrich_returns_clean_message_from_structured_error(): + # A structured GraphQL error surfaces only its clean message; the code and + # correlation_id live in extensions and never leak into user-facing text. + exc = PipefyGraphQLError( + [ + { + "message": "Invalid prompt", + "extensions": { + "code": "INVALID_PROMPT", + "correlation_id": "secret-correlation-uuid", + }, + } + ] ) behaviors = _make_behaviors(("B1", "card_created", "update_card")) result = enrich_behavior_error(exc, behaviors) @@ -209,8 +220,12 @@ def test_enrich_strips_internal_api_diagnostic_markers(): @pytest.mark.unit -def test_enrich_only_diagnostic_markers_uses_generic_fallback(): - exc = ValueError(" [code=X] [correlation_id=Y]") +def test_enrich_blank_message_uses_generic_fallback(): + # A structured error carrying a code but no human-readable message falls back + # to the generic guidance while still summarizing the behaviors that were sent. + exc = PipefyGraphQLError( + [{"message": " ", "extensions": {"code": "X", "correlation_id": "Y"}}] + ) behaviors = _make_behaviors(("B1", "card_created", "update_card")) result = enrich_behavior_error(exc, behaviors) assert "The AI behavior request failed" in result diff --git a/packages/mcp/tests/tools/test_automation_tools.py b/packages/mcp/tests/tools/test_automation_tools.py index 9c613b23..e5847e39 100644 --- a/packages/mcp/tests/tools/test_automation_tools.py +++ b/packages/mcp/tests/tools/test_automation_tools.py @@ -8,7 +8,7 @@ from mcp.shared.memory import ( create_connected_server_and_client_session as create_client_session, ) -from pipefy_sdk import PipefyClient +from pipefy_sdk import PipefyClient, PipefyGraphQLError from pipefy_mcp.tools.automation_tools import AutomationTools from pipefy_mcp.tools.tool_error_envelope import tool_error_message @@ -577,9 +577,8 @@ async def test_create_automation_error( async def test_create_automation_error_only_diagnostic_markers_uses_fallback( automation_session, mock_automation_client, extract_payload ): - mock_automation_client.create_automation.side_effect = TransportQueryError( - " [code=X] [correlation_id=Y]", - errors=[{"message": " [code=X] [correlation_id=Y]"}], + mock_automation_client.create_automation.side_effect = PipefyGraphQLError( + [{"message": " ", "extensions": {"code": "X", "correlation_id": "Y"}}] ) async with automation_session as session: @@ -605,9 +604,8 @@ async def test_create_automation_error_only_diagnostic_markers_uses_fallback( async def test_create_automation_error_only_markers_with_debug_keeps_fallback( automation_session, mock_automation_client, extract_payload ): - mock_automation_client.create_automation.side_effect = TransportQueryError( - " [code=X] [correlation_id=Y]", - errors=[{"message": " [code=X] [correlation_id=Y]"}], + mock_automation_client.create_automation.side_effect = PipefyGraphQLError( + [{"message": " ", "extensions": {"code": "X", "correlation_id": "Y"}}] ) async with automation_session as session: diff --git a/packages/mcp/tests/tools/test_portal_tool_helpers.py b/packages/mcp/tests/tools/test_portal_tool_helpers.py index c70876b4..bcf13e7d 100644 --- a/packages/mcp/tests/tools/test_portal_tool_helpers.py +++ b/packages/mcp/tests/tools/test_portal_tool_helpers.py @@ -4,6 +4,7 @@ import pytest from gql.transport.exceptions import TransportQueryError +from pipefy_sdk import PipefyGraphQLError from pipefy_sdk.exceptions import PortalPermissionError from pipefy_mcp.tools.portal_tool_helpers import ( @@ -36,19 +37,34 @@ def test_map_portal_error_permission_denied_transport_query_error() -> None: @pytest.mark.unit -def test_map_portal_error_permission_denied_internal_api_value_error() -> None: - """Internal API ValueError with [code=PERMISSION_DENIED] maps to guidance.""" - exc = ValueError("User denied [code=PERMISSION_DENIED] [correlation_id=abc-123]") +def test_map_portal_error_permission_denied_internal_api() -> None: + """A PERMISSION_DENIED GraphQL error maps to portal permission guidance.""" + exc = PipefyGraphQLError( + [ + { + "message": "User denied", + "extensions": { + "code": "PERMISSION_DENIED", + "correlation_id": "abc-123", + }, + } + ] + ) message = map_portal_error_to_message(exc) assert "create_portal" in message or "manage_portals" in message @pytest.mark.unit -def test_map_portal_error_non_permission_internal_api_value_error_strips_markers() -> ( - None -): - """Non-permission Internal API ValueError returns marker-free text.""" - exc = ValueError("Bad request [code=BAD_REQUEST] [correlation_id=abc-123]") +def test_map_portal_error_non_permission_internal_api_returns_clean_message() -> None: + """A non-permission GraphQL error surfaces its clean message (no markers).""" + exc = PipefyGraphQLError( + [ + { + "message": "Bad request", + "extensions": {"code": "BAD_REQUEST", "correlation_id": "abc-123"}, + } + ] + ) assert map_portal_error_to_message(exc) == "Bad request" diff --git a/packages/mcp/tests/tools/test_portal_tools.py b/packages/mcp/tests/tools/test_portal_tools.py index f26e487d..e89028ad 100644 --- a/packages/mcp/tests/tools/test_portal_tools.py +++ b/packages/mcp/tests/tools/test_portal_tools.py @@ -7,11 +7,11 @@ import pytest from _shared.fixture_ids import EXAMPLE_NUMERIC_ORG_ID, EXAMPLE_PIPE_REPO_ID -from gql.transport.exceptions import TransportQueryError +from gql.transport.exceptions import TransportError, TransportQueryError from mcp.shared.memory import ( create_connected_server_and_client_session as create_client_session, ) -from pipefy_sdk import PipefyClient +from pipefy_sdk import PipefyClient, PipefyGraphQLError from pipefy_sdk.exceptions import PortalPermissionError from pipefy_mcp.tools.portal_tools import PortalTools @@ -63,9 +63,13 @@ "`create_portal` or `manage_portals` from your admin." ) -_INTERNAL_API_PERMISSION_DENIED_VALUE_ERROR = ValueError( - "User does not have permission to manage portals " - "[code=PERMISSION_DENIED] [correlation_id=abc-123]" +_INTERNAL_API_PERMISSION_DENIED_ERROR = PipefyGraphQLError( + [ + { + "message": "User does not have permission to manage portals", + "extensions": {"code": "PERMISSION_DENIED", "correlation_id": "abc-123"}, + } + ] ) _PORTAL_UUID = "portal-uuid-1" @@ -1586,8 +1590,10 @@ async def test_create_sub_portal_rejects_blank_main_portal_uuid( async def test_create_sub_portal_transport_error_not_permission_envelope( portal_session, mock_portal_client, extract_payload ): + # A genuine transport failure (timeout/network), not a GraphQL error envelope: + # its message must surface verbatim and must not be mistaken for a permission denial. mock_portal_client.create_sub_portal = AsyncMock( - side_effect=TransportQueryError("failed", errors=[{"message": "timeout"}]) + side_effect=TransportError("timeout") ) async with portal_session as session: @@ -1831,7 +1837,7 @@ async def test_sub_portal_internal_api_permission_denied_returns_actionable_erro setattr( mock_portal_client, client_method, - AsyncMock(side_effect=_INTERNAL_API_PERMISSION_DENIED_VALUE_ERROR), + AsyncMock(side_effect=_INTERNAL_API_PERMISSION_DENIED_ERROR), ) async with portal_session as session: diff --git a/packages/sdk/src/pipefy_sdk/__init__.py b/packages/sdk/src/pipefy_sdk/__init__.py index ea8e38af..eacba6fd 100644 --- a/packages/sdk/src/pipefy_sdk/__init__.py +++ b/packages/sdk/src/pipefy_sdk/__init__.py @@ -11,6 +11,7 @@ filter_fields_by_definitions, skipped_field_ids, ) +from pipefy_sdk.graphql_executor import PipefyGraphQLError from pipefy_sdk.models import ( Attachment, AttachmentTarget, @@ -108,6 +109,7 @@ "PipefyClient", "PipefyEngine", "PipefyError", + "PipefyGraphQLError", "PipefyId", "PipefySettings", "TableRecordTarget", diff --git a/packages/sdk/src/pipefy_sdk/client.py b/packages/sdk/src/pipefy_sdk/client.py index 6030cdeb..64de77d3 100644 --- a/packages/sdk/src/pipefy_sdk/client.py +++ b/packages/sdk/src/pipefy_sdk/client.py @@ -19,7 +19,6 @@ GraphQLEndpoint, GraphQLExecutor, ) -from pipefy_sdk.internal_api_errors import format_internal_api_error from pipefy_sdk.models.ai_agent import ( BehaviorInput, CreateAiAgentInput, @@ -113,9 +112,7 @@ def build_endpoints( """Build one auth-less endpoint per Pipefy API endpoint from ``settings``. This is the seam that resolves each endpoint URL from settings; the endpoints - take a ready URL and stay agnostic to endpoint topology and to identity. Only - the internal endpoint carries the ``[code=…][correlation_id=…]`` error - envelope; the others leave gql exceptions untouched. + take a ready URL and stay agnostic to endpoint topology and to identity. The client telemetry headers are resolved once here from ``surface`` and the package version, then shared by all three endpoints: every endpoint targets a @@ -138,7 +135,6 @@ def build_endpoints( url=settings.internal_api_url, cache_schema=cache_schema, headers=headers, - on_graphql_error=format_internal_api_error, ), ) diff --git a/packages/sdk/src/pipefy_sdk/graphql_executor.py b/packages/sdk/src/pipefy_sdk/graphql_executor.py index c5a1a6f4..a2981673 100644 --- a/packages/sdk/src/pipefy_sdk/graphql_executor.py +++ b/packages/sdk/src/pipefy_sdk/graphql_executor.py @@ -1,9 +1,8 @@ from __future__ import annotations import asyncio -from collections.abc import Callable from dataclasses import dataclass -from typing import Any, ClassVar, NoReturn, Protocol +from typing import Any, ClassVar, Protocol from gql import Client from gql.graphql_request import GraphQLRequest @@ -14,42 +13,86 @@ @dataclass(frozen=True) -class PartialQueryResult: +class GraphQLResult: """GraphQL ``data`` plus the raw per-node error dicts from one response. - Owned by the executor so services and their tests never touch gql objects. - ``data`` is always a dict; a fully null response yields an empty one. + A GraphQL response can carry readable ``data`` and per-node ``errors`` at the + same time, so the primitive hands both back together. Owned by the executor + so services and their tests never touch gql objects. ``data`` is always a + dict; a fully null response yields an empty one. """ data: dict[str, Any] errors: list[dict[str, Any]] +def _graphql_error_message(errors: list[dict[str, Any]]) -> str: + """Join the human-readable messages from raw GraphQL error dicts. + + Tolerates a non-conforming ``errors`` element that is not a dict (a server may + return a bare string), matching the sibling extractors in the MCP tool layer. + """ + parts = [ + (err.get("message") if isinstance(err, dict) else str(err)) or "Unknown error" + for err in errors + ] + return "; ".join(parts) or "Query failed." + + +class PipefyGraphQLError(Exception): + """A GraphQL response came back carrying ``errors``. + + Owned by the SDK so callers catch one error type instead of a gql transport + exception. ``errors`` is the raw per-node error dict list (each with its own + ``message`` and ``extensions``); consumers read codes and correlation ids off + that structure rather than parsing the message string. + """ + + def __init__(self, errors: list[dict[str, Any]]) -> None: + self.errors = errors + super().__init__(_graphql_error_message(errors)) + + +def data_or_raise(result: GraphQLResult) -> dict: + """Return ``result.data``, or raise :class:`PipefyGraphQLError` if it held errors. + + The raise-on-error decision as a pure function: it is what the query and + mutation callers want (data or an exception, nothing in between) without the + seam itself deciding that a response with errors is a failure. Services that + handle partial success skip it and read ``result.errors`` directly. + """ + if result.errors: + raise PipefyGraphQLError(result.errors) + return result.data + + class GraphQLExecutor(Protocol): """The GraphQL execution seam services depend on. - Narrow by design: it exposes only the operation services need and leaks - nothing about the httpx/gql transport. Services receive an implementation - through their constructor and call ``execute_query``; tests inject a fake. + Narrow by design: it exposes only what services need and leaks nothing about + the httpx/gql transport. :meth:`execute` returns an owned + :class:`GraphQLResult`, and :meth:`execute_query` raises an owned + :class:`PipefyGraphQLError`. Services receive an implementation through their + constructor and call one of two methods; tests inject a fake. + + :meth:`execute` is the primitive: it performs the request and returns + ``data`` and ``errors`` together, deciding nothing about what the errors + mean. :meth:`execute_query` is the raise-on-error convenience layered over it + (via :func:`data_or_raise`), for the callers that want data or an exception + and nothing in between. A service that needs partial-success handling calls + :meth:`execute` and hands the errors to its own classifier. + ``query`` is a parsed ``DocumentNode``: callers build one with ``gql()`` (the raw ``execute_graphql`` passthrough parses its string before reaching here). """ - async def execute_query( + async def execute( self, query: DocumentNode, variables: dict[str, Any] - ) -> dict: ... - + ) -> GraphQLResult: ... -class PartialGraphQLExecutor(GraphQLExecutor, Protocol): - """:class:`GraphQLExecutor` plus a partial-tolerant execute path. - - For services whose queries mix ``data`` with per-node ``errors`` in one - response; everything else depends on the narrower protocol. - """ - - async def execute_query_allow_partial( + async def execute_query( self, query: DocumentNode, variables: dict[str, Any] - ) -> PartialQueryResult: ... + ) -> dict: ... def _graphql_request_with_variables( @@ -66,11 +109,11 @@ class GraphQLEndpoint: """The shared, auth-less half of a Pipefy GraphQL connection. Holds everything about one endpoint that does not depend on the caller's - identity: its URL, telemetry headers, error formatting, and the introspected - schema cache. Built once and shared across identities; the execute methods take - the ``auth`` per call so the same endpoint (and its one schema cache) serves - every caller. A fresh transport is opened per call so concurrent requests never - share mutable transport state (avoids ``TransportAlreadyConnected``). + identity: its URL, telemetry headers, and the introspected schema cache. Built + once and shared across identities; the execute methods take the ``auth`` per + call so the same endpoint (and its one schema cache) serves every caller. A + fresh transport is opened per call so concurrent requests never share mutable + transport state (avoids ``TransportAlreadyConnected``). """ GRAPHQL_REQUEST_TIMEOUT_SECONDS: ClassVar[int] = 30 @@ -81,17 +124,11 @@ def __init__( url: str, cache_schema: bool = False, headers: dict[str, str] | None = None, - on_graphql_error: Callable[[list[dict]], str] | None = None, ) -> None: # Fully resolved endpoint URL; the endpoint does no settings resolution itself. self._graphql_url = url self._cache_schema = cache_schema self._headers = headers - # When set, ``TransportQueryError`` is converted to ``ValueError`` using the - # formatter's output. Used by the Internal API endpoint to surface its - # ``[code=…] [correlation_id=…]`` envelope; ``None`` leaves gql exceptions - # untouched (the public endpoint's behaviour). - self._on_graphql_error = on_graphql_error # Caches the introspected schema so it is fetched once, not per Client. self._fetched_gql_schema: GraphQLSchema | None = None self._fetched_gql_schema_lock = asyncio.Lock() @@ -146,64 +183,54 @@ async def _execute_request( ) as session: return await session.execute(request) - def _reraise_graphql_error(self, exc: TransportQueryError) -> NoReturn: - if self._on_graphql_error is None: - raise exc - raise ValueError(self._on_graphql_error(exc.errors or [])) from exc - async def execute( self, query: DocumentNode, variables: dict[str, Any], *, auth: Auth - ) -> dict: - """Execute a GraphQL query/mutation with variables under ``auth``. - - All-or-nothing: any GraphQL ``errors`` raise and partial ``data`` is - discarded; for responses that mix both, use :meth:`execute_allow_partial`. + ) -> GraphQLResult: + """Execute a query and return its ``data`` and raw ``errors`` together. + + The primitive: it performs the effect and decides nothing about what the + errors mean. gql raises on any ``errors`` but attaches the partial ``data`` + and the raw error dicts to the exception; this rebuilds them into a + :class:`GraphQLResult`. A fully null response yields ``data={}`` with its + errors preserved. Genuine transport/network failures (timeouts, non-2xx) + still raise. """ try: - return await self._execute_request(query, variables, auth=auth) + data = await self._execute_request(query, variables, auth=auth) except TransportQueryError as exc: - self._reraise_graphql_error(exc) + return GraphQLResult(data=exc.data or {}, errors=list(exc.errors or [])) + return GraphQLResult(data=data, errors=[]) - async def execute_allow_partial( + async def execute_query( self, query: DocumentNode, variables: dict[str, Any], *, auth: Auth - ) -> PartialQueryResult: - """Execute a query preserving partial ``data`` alongside GraphQL ``errors``. - - gql raises on any ``errors`` but attaches the partial ``data`` and the raw - error dicts to the exception; this rebuilds them into a - :class:`PartialQueryResult`. A fully null response yields ``data={}`` with - its errors preserved: whether an empty result is a failure is the caller's - decision, not the transport's. + ) -> dict: + """Run :meth:`execute` and return ``data``, raising if the response held errors. + + The raise-on-error convenience the query/mutation tools use: they want + ``data`` or an exception, not partial success. On GraphQL ``errors`` it + raises :class:`PipefyGraphQLError` (see :func:`data_or_raise`). """ - try: - data = await self._execute_request(query, variables, auth=auth) - except TransportQueryError as exc: - return PartialQueryResult( - data=exc.data or {}, errors=list(exc.errors or []) - ) - return PartialQueryResult(data=data, errors=[]) + return data_or_raise(await self.execute(query, variables, auth=auth)) @dataclass(frozen=True) class AuthenticatedExecutor: """A :class:`GraphQLEndpoint` bound to one identity's ``auth``. - The cheap per-session executor: it implements :class:`PartialGraphQLExecutor` - by delegating to the shared endpoint with its own ``auth``. Cheap to build one + The cheap per-session executor: it implements :class:`GraphQLExecutor` by + delegating to the shared endpoint with its own ``auth``. Cheap to build one per request; the endpoint (and its schema cache) is reused across all of them. """ endpoint: GraphQLEndpoint auth: Auth - async def execute_query( + async def execute( self, query: DocumentNode, variables: dict[str, Any] - ) -> dict: + ) -> GraphQLResult: return await self.endpoint.execute(query, variables, auth=self.auth) - async def execute_query_allow_partial( + async def execute_query( self, query: DocumentNode, variables: dict[str, Any] - ) -> PartialQueryResult: - return await self.endpoint.execute_allow_partial( - query, variables, auth=self.auth - ) + ) -> dict: + return await self.endpoint.execute_query(query, variables, auth=self.auth) diff --git a/packages/sdk/src/pipefy_sdk/internal_api_errors.py b/packages/sdk/src/pipefy_sdk/internal_api_errors.py deleted file mode 100644 index 3779beac..00000000 --- a/packages/sdk/src/pipefy_sdk/internal_api_errors.py +++ /dev/null @@ -1,21 +0,0 @@ -"""Error envelope for the internal_api GraphQL endpoint. - -The internal_api executor decorates each GraphQL error with ``[code=…]`` / -``[correlation_id=…]`` suffixes drawn from ``extensions``. Service-layer tests -assert the fully suffixed text. -""" - -from __future__ import annotations - - -def format_internal_api_error(errors: list[dict]) -> str: - parts: list[str] = [] - for err in errors: - msg = err.get("message", "Unknown error") - ext = err.get("extensions", {}) - code = ext.get("code", "") - corr = ext.get("correlation_id", "") - suffix = f" [code={code}]" if code else "" - suffix += f" [correlation_id={corr}]" if corr else "" - parts.append(f"{msg}{suffix}") - return "; ".join(parts) diff --git a/packages/sdk/src/pipefy_sdk/services/observability_service.py b/packages/sdk/src/pipefy_sdk/services/observability_service.py index 2f760f4e..d924e5e3 100644 --- a/packages/sdk/src/pipefy_sdk/services/observability_service.py +++ b/packages/sdk/src/pipefy_sdk/services/observability_service.py @@ -8,7 +8,7 @@ import httpx from openpyxl.utils.exceptions import InvalidFileException -from pipefy_sdk.graphql_executor import PartialGraphQLExecutor, PartialQueryResult +from pipefy_sdk.graphql_executor import GraphQLExecutor, GraphQLResult from pipefy_sdk.queries.observability_queries import ( CREATE_AUTOMATION_JOBS_EXPORT_MUTATION, GET_AGENTS_USAGE_QUERY, @@ -163,7 +163,7 @@ def _partial_error(err: dict[str, Any]) -> dict[str, Any]: } -def _normalize_execution_metrics_result(result: PartialQueryResult) -> dict[str, Any]: +def _normalize_execution_metrics_result(result: GraphQLResult) -> dict[str, Any]: """Split a partial ``automations.executionMetrics`` response into nodes and errors. The sole failure decision for this query: a missing or null ``automations`` @@ -189,7 +189,7 @@ def _normalize_execution_metrics_result(result: PartialQueryResult) -> dict[str, class ObservabilityService: """Reads for AI agent logs, automation logs, usage stats, and credit dashboard.""" - def __init__(self, *, executor: PartialGraphQLExecutor) -> None: + def __init__(self, *, executor: GraphQLExecutor) -> None: self._executor = executor async def get_automation_execution_metrics( @@ -245,7 +245,7 @@ async def get_automation_execution_metrics( sort_by=sort_by, sort_order=sort_order, ) - result = await self._executor.execute_query_allow_partial( + result = await self._executor.execute( GET_AUTOMATION_EXECUTION_METRICS_QUERY, variables ) return _normalize_execution_metrics_result(result) diff --git a/packages/sdk/src/pipefy_sdk/services/portal_service.py b/packages/sdk/src/pipefy_sdk/services/portal_service.py index 92d8b2c7..a661ac1a 100644 --- a/packages/sdk/src/pipefy_sdk/services/portal_service.py +++ b/packages/sdk/src/pipefy_sdk/services/portal_service.py @@ -6,11 +6,10 @@ import logging from typing import Any -from gql.transport.exceptions import TransportQueryError from graphql import DocumentNode from pipefy_sdk.exceptions import PortalPermissionError -from pipefy_sdk.graphql_executor import GraphQLExecutor +from pipefy_sdk.graphql_executor import GraphQLExecutor, PipefyGraphQLError from pipefy_sdk.models.portal import ( CreatePortalElementInput, CreatePortalInput, @@ -60,7 +59,7 @@ def _with_uuid_alias(record: dict[str, Any]) -> dict[str, Any]: def _map_portal_permission_error( - exc: TransportQueryError, + exc: PipefyGraphQLError, ) -> PortalPermissionError | None: """Return ``PortalPermissionError`` only for PERMISSION_DENIED; else ``None``.""" for err in exc.errors or []: @@ -114,38 +113,26 @@ def _normalize_portal_data_sources( return normalized -async def _execute_interfaces_query_with_portal_errors( +async def _execute_query_with_portal_errors( execute: Any, query: Any, variables: dict[str, Any], ) -> dict[str, Any]: - """Run an Interfaces operation and map portal permission failures.""" + """Run a portal operation and map PERMISSION_DENIED to ``PortalPermissionError``. + + Serves both the Interfaces and Internal API endpoints: each raises + ``PipefyGraphQLError`` on GraphQL errors, and the PERMISSION_DENIED code lives + in the structured ``errors`` regardless of endpoint. + """ try: return await execute(query, variables) - except TransportQueryError as exc: + except PipefyGraphQLError as exc: permission_error = _map_portal_permission_error(exc) if permission_error is not None: raise permission_error from exc raise -_INTERNAL_API_PERMISSION_DENIED_MARKER = "[code=PERMISSION_DENIED]" - - -async def _execute_internal_api_query_with_portal_errors( - execute: Any, - query: str, - variables: dict[str, Any], -) -> dict[str, Any]: - """Run an Internal API operation and map portal permission failures.""" - try: - return await execute(query, variables) - except ValueError as exc: - if _INTERNAL_API_PERMISSION_DENIED_MARKER in str(exc): - raise PortalPermissionError(_PORTAL_PERMISSION_MESSAGE) from exc - raise - - def _graphql_create_element_input( validated: CreatePortalElementInput, ) -> dict[str, Any]: @@ -320,7 +307,7 @@ async def create_portal(self, organization_uuid: str | int) -> dict[str, Any]: self._public_executor.execute_query, portal_input.organization_uuid, ) - data = await _execute_interfaces_query_with_portal_errors( + data = await _execute_query_with_portal_errors( self.execute_interfaces_query, FIND_OR_CREATE_PORTAL_MUTATION, {"input": {"orgUuid": resolved_org_uuid, "subType": "portal"}}, @@ -366,7 +353,7 @@ async def update_portal( by_alias=True, ) } - data = await _execute_interfaces_query_with_portal_errors( + data = await _execute_query_with_portal_errors( self.execute_interfaces_query, UPDATE_INTERFACE_MUTATION, variables, @@ -383,7 +370,7 @@ async def delete_portal(self, interface_uuid: str) -> dict[str, Any]: Args: interface_uuid: Portal interface UUID. """ - return await _execute_interfaces_query_with_portal_errors( + return await _execute_query_with_portal_errors( self.execute_interfaces_query, DELETE_INTERFACE_MUTATION, {"input": {"interface_uuid": interface_uuid}}, @@ -416,7 +403,7 @@ async def create_portal_page( page_input["description"] = description if index is not None: page_input["index"] = index - data = await _execute_interfaces_query_with_portal_errors( + data = await _execute_query_with_portal_errors( self.execute_interfaces_query, CREATE_PAGE_MUTATION, {"input": page_input}, @@ -455,7 +442,7 @@ async def update_portal_page( page_input["description"] = description if index is not None: page_input["index"] = index - data = await _execute_interfaces_query_with_portal_errors( + data = await _execute_query_with_portal_errors( self.execute_interfaces_query, UPDATE_PAGE_MUTATION, {"input": page_input}, @@ -475,7 +462,7 @@ async def delete_portal_page( interface_uuid: Parent portal interface UUID. page_id: Page UUID. """ - return await _execute_interfaces_query_with_portal_errors( + return await _execute_query_with_portal_errors( self.execute_interfaces_query, DELETE_PAGE_MUTATION, {"input": {"interface_uuid": interface_uuid, "page_id": page_id}}, @@ -490,7 +477,7 @@ async def sort_portal_pages( interface_uuid: Parent portal interface UUID. page_ids: Ordered list of page UUIDs. """ - return await _execute_interfaces_query_with_portal_errors( + return await _execute_query_with_portal_errors( self.execute_interfaces_query, SORT_PAGES_MUTATION, {"input": {"interface_uuid": interface_uuid, "page_ids": page_ids}}, @@ -505,7 +492,7 @@ async def update_portal_page_layout( page_id: Page UUID (no parent ``interface_uuid`` on this mutation). layout: Layout JSON as required by ``updatePageLayout``. """ - return await _execute_interfaces_query_with_portal_errors( + return await _execute_query_with_portal_errors( self.execute_interfaces_query, UPDATE_PAGE_LAYOUT_MUTATION, { @@ -549,7 +536,7 @@ async def create_portal_element( "layout": layout, } ) - data = await _execute_interfaces_query_with_portal_errors( + data = await _execute_query_with_portal_errors( self.execute_interfaces_query, CREATE_ELEMENT_MUTATION, {"input": _graphql_create_element_input(validated)}, @@ -594,7 +581,7 @@ async def update_portal_element( "editable": editable, } ) - data = await _execute_interfaces_query_with_portal_errors( + data = await _execute_query_with_portal_errors( self.execute_interfaces_query, UPDATE_ELEMENT_MUTATION, {"input": _graphql_update_element_input(validated)}, @@ -623,7 +610,7 @@ async def delete_portal_element( element_id: Element UUID. page_id: Parent page UUID. """ - return await _execute_interfaces_query_with_portal_errors( + return await _execute_query_with_portal_errors( self.execute_interfaces_query, DELETE_ELEMENT_MUTATION, {"input": {"element_id": element_id, "page_id": page_id}}, @@ -646,7 +633,7 @@ async def duplicate_portal_element( portal_uuid: Portal interface UUID that owns the page. page_id: Page UUID that contains the element. """ - data = await _execute_interfaces_query_with_portal_errors( + data = await _execute_query_with_portal_errors( self.execute_interfaces_query, DUPLICATE_ELEMENT_MUTATION, { @@ -677,7 +664,7 @@ async def create_sub_portal( portal_input: dict[str, Any] = {"mainPortalUuid": main_portal_uuid} if name is not None: portal_input["name"] = name - data = await _execute_interfaces_query_with_portal_errors( + data = await _execute_query_with_portal_errors( self.execute_interfaces_query, CREATE_SUB_PORTAL_MUTATION, {"input": portal_input}, @@ -701,7 +688,7 @@ async def update_sub_portal_element( element_id: Page element UUID (e.g. templated ``forms`` slot). sub_portal_uuid: Sub-portal UUID to wire to the element. """ - return await _execute_internal_api_query_with_portal_errors( + return await _execute_query_with_portal_errors( self.execute_internal_api_query, UPDATE_SUB_PORTAL_ELEMENT_MUTATION, { @@ -747,7 +734,7 @@ async def unpublish_sub_portal( portal_uuid: Main portal interface UUID. element_id: Page element UUID. """ - return await _execute_internal_api_query_with_portal_errors( + return await _execute_query_with_portal_errors( self.execute_internal_api_query, UPDATE_SUB_PORTAL_ELEMENT_MUTATION, { @@ -770,7 +757,7 @@ async def delete_sub_portal_element( portal_uuid: Main portal interface UUID. element_id: Page element UUID. """ - return await _execute_internal_api_query_with_portal_errors( + return await _execute_query_with_portal_errors( self.execute_internal_api_query, DELETE_SUB_PORTAL_ELEMENT_MUTATION, {"input": {"portalUuid": portal_uuid, "elementId": element_id}}, @@ -782,7 +769,7 @@ async def delete_sub_portal(self, uuid: str) -> dict[str, Any]: Args: uuid: Sub-portal UUID. """ - return await _execute_internal_api_query_with_portal_errors( + return await _execute_query_with_portal_errors( self.execute_internal_api_query, DELETE_SUB_PORTAL_INTERFACE_MUTATION, {"input": {"uuid": uuid}}, diff --git a/packages/sdk/src/pipefy_sdk/services/schema_introspection_service.py b/packages/sdk/src/pipefy_sdk/services/schema_introspection_service.py index ebc5c7be..2d929be6 100644 --- a/packages/sdk/src/pipefy_sdk/services/schema_introspection_service.py +++ b/packages/sdk/src/pipefy_sdk/services/schema_introspection_service.py @@ -7,10 +7,9 @@ from typing import Any from gql import gql -from gql.transport.exceptions import TransportQueryError from graphql import GraphQLError, GraphQLSyntaxError -from pipefy_sdk.graphql_executor import GraphQLExecutor +from pipefy_sdk.graphql_executor import GraphQLExecutor, PipefyGraphQLError from pipefy_sdk.queries.introspection_queries import ( INTROSPECT_MUTATION_QUERY, INTROSPECT_QUERY_QUERY, @@ -229,7 +228,7 @@ async def _detect_root_type_mismatch_hint(self, error_message: str) -> str | Non f"not a {current_type.lower()}. " f"Use a {other_type.lower()} operation instead." ) - except (TransportQueryError, GraphQLError, KeyError, TypeError): + except (PipefyGraphQLError, GraphQLError, KeyError, TypeError): return None except Exception: logger.exception( @@ -256,7 +255,7 @@ async def execute_graphql( return {"error": str(exc)} try: return await self._executor.execute_query(document, variables or {}) - except TransportQueryError as exc: + except PipefyGraphQLError as exc: errors = list(exc.errors) if exc.errors else [{"message": str(exc)}] for err in errors: if isinstance(err, dict): diff --git a/packages/sdk/src/pipefy_sdk/services/webhook_service.py b/packages/sdk/src/pipefy_sdk/services/webhook_service.py index e87e7d64..cc02e904 100644 --- a/packages/sdk/src/pipefy_sdk/services/webhook_service.py +++ b/packages/sdk/src/pipefy_sdk/services/webhook_service.py @@ -5,10 +5,9 @@ import logging from typing import Any -from gql.transport.exceptions import TransportQueryError from pipefy_infra import security -from pipefy_sdk.graphql_executor import GraphQLExecutor +from pipefy_sdk.graphql_executor import GraphQLExecutor, PipefyGraphQLError from pipefy_sdk.queries.webhook_queries import ( CREATE_AND_SEND_INBOX_EMAIL_MUTATION, CREATE_WEBHOOK_MUTATION, @@ -98,7 +97,7 @@ async def _resolve_repo_id( pipe_id = pipe_obj.get("id") if isinstance(pipe_obj, dict) else None if pipe_id is not None: return {**attrs, "repoId": str(pipe_id)} - except (TransportQueryError, KeyError, TypeError): + except (PipefyGraphQLError, KeyError, TypeError): logger.debug( "Could not auto-resolve repoId for card %s", card_id, diff --git a/packages/sdk/tests/_shared/mock_clients.py b/packages/sdk/tests/_shared/mock_clients.py index 7e25057f..15d7c398 100644 --- a/packages/sdk/tests/_shared/mock_clients.py +++ b/packages/sdk/tests/_shared/mock_clients.py @@ -8,28 +8,27 @@ from unittest.mock import AsyncMock, MagicMock -from pipefy_sdk.graphql_executor import PartialGraphQLExecutor, PartialQueryResult +from pipefy_sdk.graphql_executor import GraphQLExecutor, GraphQLResult def mock_executor( return_value: dict | None = None, *, side_effect=None, - partial_result: PartialQueryResult | None = None, - partial_side_effect=None, + execute_result: GraphQLResult | None = None, + execute_side_effect=None, ) -> MagicMock: - """A MagicMock standing in for a :class:`PartialGraphQLExecutor`. + """A MagicMock standing in for a :class:`GraphQLExecutor`. - Spec'd on the wide protocol so one fake serves every service. - ``return_value``/``side_effect`` drive ``execute_query``; - ``partial_result``/``partial_side_effect`` drive - ``execute_query_allow_partial``. Both are stubbed explicitly: an - auto-created method would resolve to a bare MagicMock and feed services - silent garbage instead of a failure. + ``return_value``/``side_effect`` drive ``execute_query``, the raise-on-error + convenience most services call. ``execute_result``/``execute_side_effect`` + drive ``execute``, the primitive that hands back data and errors together. + Both are stubbed explicitly: an auto-created method would resolve to a bare + MagicMock and feed services silent garbage instead of a failure. """ - mock = MagicMock(spec=PartialGraphQLExecutor) + mock = MagicMock(spec=GraphQLExecutor) mock.execute_query = AsyncMock(return_value=return_value, side_effect=side_effect) - mock.execute_query_allow_partial = AsyncMock( - return_value=partial_result, side_effect=partial_side_effect + mock.execute = AsyncMock( + return_value=execute_result, side_effect=execute_side_effect ) return mock diff --git a/packages/sdk/tests/services/pipefy/test_observability_service.py b/packages/sdk/tests/services/pipefy/test_observability_service.py index 50da92df..cf7d8ae7 100644 --- a/packages/sdk/tests/services/pipefy/test_observability_service.py +++ b/packages/sdk/tests/services/pipefy/test_observability_service.py @@ -10,7 +10,7 @@ from gql.transport.exceptions import TransportQueryError from openpyxl import Workbook -from pipefy_sdk.graphql_executor import PartialQueryResult +from pipefy_sdk.graphql_executor import GraphQLResult from pipefy_sdk.queries.observability_queries import ( CREATE_AUTOMATION_JOBS_EXPORT_MUTATION, GET_AGENTS_USAGE_QUERY, @@ -34,7 +34,7 @@ def _make_service(return_value): def _make_partial_service(partial_result): - executor = mock_executor(partial_result=partial_result) + executor = mock_executor(execute_result=partial_result) service = ObservabilityService(executor=executor) return service, executor @@ -559,7 +559,7 @@ def _denied_error(automation_id: str) -> dict: @pytest.mark.asyncio async def test_get_automation_execution_metrics_all_permitted(): """Full success: all requested automations return metrics, no partial errors.""" - partial_result = PartialQueryResult( + partial_result = GraphQLResult( data={ "automations": { "pageInfo": {"hasNextPage": False, "endCursor": "cur-2"}, @@ -577,8 +577,8 @@ async def test_get_automation_execution_metrics_all_permitted(): "3", ["25", "107"], repo_id="16", period="TWENTY_FOUR_HOURS" ) - executor.execute_query_allow_partial.assert_awaited_once() - query, variables = executor.execute_query_allow_partial.call_args[0] + executor.execute.assert_awaited_once() + query, variables = executor.execute.call_args[0] assert query is GET_AUTOMATION_EXECUTION_METRICS_QUERY assert variables == { "organizationId": "3", @@ -597,7 +597,7 @@ async def test_get_automation_execution_metrics_all_permitted(): @pytest.mark.asyncio async def test_get_automation_execution_metrics_partial_permission_denied(): """Partial success: permitted nodes returned, denied ids surfaced in partial_errors.""" - partial_result = PartialQueryResult( + partial_result = GraphQLResult( data={"automations": {"edges": [{"node": _metrics_node("25", total_runs=3)}]}}, errors=[_denied_error("124"), _denied_error("65")], ) @@ -620,7 +620,7 @@ async def test_get_automation_execution_metrics_partial_permission_denied(): "correlation_id": "c86ccfa6", }, ] - _, variables = executor.execute_query_allow_partial.call_args[0] + _, variables = executor.execute.call_args[0] assert "repoId" not in variables @@ -628,7 +628,7 @@ async def test_get_automation_execution_metrics_partial_permission_denied(): @pytest.mark.asyncio async def test_get_automation_execution_metrics_all_denied_stays_partial(): """Every requested id denied but the connection is present: empty list, not a raise.""" - partial_result = PartialQueryResult( + partial_result = GraphQLResult( data={"automations": {"edges": []}}, errors=[_denied_error("124"), _denied_error("65")], ) @@ -644,7 +644,7 @@ async def test_get_automation_execution_metrics_all_denied_stays_partial(): @pytest.mark.asyncio async def test_get_automation_execution_metrics_null_connection_raises(): """A null automations connection (lookup failed) is a total failure, not empty success.""" - partial_result = PartialQueryResult( + partial_result = GraphQLResult( data={"automations": None}, errors=[ { @@ -663,7 +663,7 @@ async def test_get_automation_execution_metrics_null_connection_raises(): @pytest.mark.asyncio async def test_get_automation_execution_metrics_null_connection_without_errors_raises(): """A null connection raises even when the response carries no error to quote.""" - partial_result = PartialQueryResult(data={"automations": None}, errors=[]) + partial_result = GraphQLResult(data={"automations": None}, errors=[]) service, _ = _make_partial_service(partial_result) with pytest.raises(ValueError, match="Query failed."): @@ -674,7 +674,7 @@ async def test_get_automation_execution_metrics_null_connection_without_errors_r @pytest.mark.asyncio async def test_get_automation_execution_metrics_empty_data_raises(): """A fully null response (executor hands back empty data) fails through the same decision.""" - partial_result = PartialQueryResult( + partial_result = GraphQLResult( data={}, errors=[{"message": "Couldn't find Organization with 'id'=999"}], ) @@ -688,7 +688,7 @@ async def test_get_automation_execution_metrics_empty_data_raises(): @pytest.mark.asyncio async def test_get_automation_execution_metrics_without_ids_omits_filter(): """Omitting automation_ids drops the automationIds variable so the query returns all.""" - partial_result = PartialQueryResult( + partial_result = GraphQLResult( data={ "automations": { "edges": [ @@ -703,7 +703,7 @@ async def test_get_automation_execution_metrics_without_ids_omits_filter(): result = await service.get_automation_execution_metrics("3") - _, variables = executor.execute_query_allow_partial.call_args[0] + _, variables = executor.execute.call_args[0] assert "automationIds" not in variables assert variables == {"organizationId": "3", "period": "SIXTY_MINUTES", "first": 50} assert [a["id"] for a in result["automations"]] == ["25", "107"] @@ -713,7 +713,7 @@ async def test_get_automation_execution_metrics_without_ids_omits_filter(): @pytest.mark.asyncio async def test_get_automation_execution_metrics_paginates_with_first_and_after(): """first and after are forwarded; after is omitted when not supplied.""" - partial_result = PartialQueryResult( + partial_result = GraphQLResult( data={ "automations": { "pageInfo": {"hasNextPage": True, "endCursor": "cur-1"}, @@ -728,7 +728,7 @@ async def test_get_automation_execution_metrics_paginates_with_first_and_after() "3", first=50, after="cur-0" ) - _, variables = executor.execute_query_allow_partial.call_args[0] + _, variables = executor.execute.call_args[0] assert variables["first"] == 50 assert variables["after"] == "cur-0" assert result["page_info"] == {"hasNextPage": True, "endCursor": "cur-1"} @@ -738,7 +738,7 @@ async def test_get_automation_execution_metrics_paginates_with_first_and_after() @pytest.mark.asyncio async def test_get_automation_execution_metrics_forwards_all_filters(): """search, active, event_id, action_ids, and sort_by/order map onto the variables.""" - partial_result = PartialQueryResult( + partial_result = GraphQLResult( data={"automations": {"edges": [{"node": _metrics_node("25", total_runs=3)}]}}, errors=[], ) @@ -754,7 +754,7 @@ async def test_get_automation_execution_metrics_forwards_all_filters(): sort_order="asc", ) - _, variables = executor.execute_query_allow_partial.call_args[0] + _, variables = executor.execute.call_args[0] assert variables["actionIds"] == ["9", "10"] assert variables["eventId"] == "card_moved" assert variables["active"] is True @@ -766,7 +766,7 @@ async def test_get_automation_execution_metrics_forwards_all_filters(): @pytest.mark.asyncio async def test_get_automation_execution_metrics_omits_unset_filters(): """Unset optional filters are absent from the variables (never sent as nulls).""" - partial_result = PartialQueryResult( + partial_result = GraphQLResult( data={"automations": {"edges": []}}, errors=[], ) @@ -774,7 +774,7 @@ async def test_get_automation_execution_metrics_omits_unset_filters(): await service.get_automation_execution_metrics("3") - _, variables = executor.execute_query_allow_partial.call_args[0] + _, variables = executor.execute.call_args[0] for absent in ("actionIds", "eventId", "active", "search", "sort"): assert absent not in variables @@ -783,7 +783,7 @@ async def test_get_automation_execution_metrics_omits_unset_filters(): @pytest.mark.asyncio async def test_get_automation_execution_metrics_partial_sort_criteria(): """sort_by alone yields a sort input with only the `by` field set.""" - partial_result = PartialQueryResult( + partial_result = GraphQLResult( data={"automations": {"edges": []}}, errors=[], ) @@ -791,5 +791,5 @@ async def test_get_automation_execution_metrics_partial_sort_criteria(): await service.get_automation_execution_metrics("3", sort_by="created_at") - _, variables = executor.execute_query_allow_partial.call_args[0] + _, variables = executor.execute.call_args[0] assert variables["sort"] == {"by": "created_at"} diff --git a/packages/sdk/tests/services/pipefy/test_portal_service.py b/packages/sdk/tests/services/pipefy/test_portal_service.py index 98fde8ea..beb0eb99 100644 --- a/packages/sdk/tests/services/pipefy/test_portal_service.py +++ b/packages/sdk/tests/services/pipefy/test_portal_service.py @@ -16,10 +16,10 @@ ) from _shared.mock_clients import mock_executor from gql import gql -from gql.transport.exceptions import TransportQueryError from pydantic import ValidationError from pipefy_sdk.exceptions import PortalPermissionError +from pipefy_sdk.graphql_executor import PipefyGraphQLError from pipefy_sdk.queries.observability_queries import RESOLVE_ORGANIZATION_UUID_QUERY from pipefy_sdk.queries.portal_queries import GET_PORTAL_QUERY, LIST_PORTALS_QUERY from pipefy_sdk.services.portal_service import PortalService @@ -441,9 +441,8 @@ def _make_portal_service_with_clients( } } -_PERMISSION_DENIED_ERROR = TransportQueryError( - "GraphQL request failed", - errors=[ +_PERMISSION_DENIED_ERROR = PipefyGraphQLError( + [ { "message": "Permission Denied", "extensions": {"code": "PERMISSION_DENIED"}, @@ -970,13 +969,12 @@ async def test_create_portal_element_graphql_error_is_not_portal_permission_erro _CREATE_ELEMENT_RESPONSE, ) interfaces_executor.execute_query = AsyncMock( - side_effect=TransportQueryError( - "invalid", - errors=[{"message": "Variable $input was provided invalid value"}], + side_effect=PipefyGraphQLError( + [{"message": "Variable $input was provided invalid value"}], ) ) - with pytest.raises(TransportQueryError): + with pytest.raises(PipefyGraphQLError): await service.create_portal_element( _PAGE_ID, type="link", @@ -1258,9 +1256,16 @@ async def test_delete_sub_portal_calls_delete_sub_portal_interface() -> None: assert result == internal_response -_INTERNAL_API_PERMISSION_DENIED_VALUE_ERROR = ValueError( - "User does not have permission to manage portals " - "[code=PERMISSION_DENIED] [correlation_id=abc-123]" +_INTERNAL_API_PERMISSION_DENIED_ERROR = PipefyGraphQLError( + [ + { + "message": "User does not have permission to manage portals", + "extensions": { + "code": "PERMISSION_DENIED", + "correlation_id": "abc-123", + }, + } + ], ) @@ -1295,7 +1300,7 @@ async def test_sub_portal_internal_api_permission_denied_surfaces_actionable_mes """PERMISSION_DENIED from the Internal API maps to portal permission guidance.""" service, _interfaces_client, internal_client = _make_portal_service_with_clients() internal_client.execute_query = AsyncMock( - side_effect=_INTERNAL_API_PERMISSION_DENIED_VALUE_ERROR + side_effect=_INTERNAL_API_PERMISSION_DENIED_ERROR ) with pytest.raises(PortalPermissionError, match=r"(create_portal|manage_portals)"): @@ -1304,13 +1309,15 @@ async def test_sub_portal_internal_api_permission_denied_surfaces_actionable_mes @pytest.mark.unit @pytest.mark.asyncio -async def test_sub_portal_internal_api_non_permission_value_error_propagates() -> None: - """Non-permission Internal API ValueError must not become PortalPermissionError.""" +async def test_sub_portal_internal_api_non_permission_error_propagates() -> None: + """Non-permission Internal API errors must not become PortalPermissionError.""" service, _interfaces_client, internal_client = _make_portal_service_with_clients() - bad_request = ValueError("Bad request [code=BAD_REQUEST] [correlation_id=abc-123]") + bad_request = PipefyGraphQLError( + [{"message": "Bad request", "extensions": {"code": "BAD_REQUEST"}}] + ) internal_client.execute_query = AsyncMock(side_effect=bad_request) - with pytest.raises(ValueError, match="Bad request"): + with pytest.raises(PipefyGraphQLError, match="Bad request"): await service.update_sub_portal_element( _MAIN_PORTAL_UUID, _FORMS_ELEMENT_ID, diff --git a/packages/sdk/tests/services/pipefy/test_schema_introspection_service.py b/packages/sdk/tests/services/pipefy/test_schema_introspection_service.py index 12b4182b..c1afea2a 100644 --- a/packages/sdk/tests/services/pipefy/test_schema_introspection_service.py +++ b/packages/sdk/tests/services/pipefy/test_schema_introspection_service.py @@ -2,9 +2,9 @@ import pytest from _shared.mock_clients import mock_executor -from gql.transport.exceptions import TransportQueryError from graphql import GraphQLError +from pipefy_sdk.graphql_executor import PipefyGraphQLError from pipefy_sdk.queries.introspection_queries import ( INTROSPECT_MUTATION_QUERY, INTROSPECT_QUERY_QUERY, @@ -695,17 +695,16 @@ async def test_execute_graphql_valid_mutation_returns_data(): @pytest.mark.unit @pytest.mark.asyncio async def test_execute_graphql_surfaces_graphql_errors_from_transport(): - """TransportQueryError from the GraphQL layer is turned into a clear errors payload.""" + """A PipefyGraphQLError from the seam is turned into a clear errors payload.""" async def raise_transport_error(*_args, **_kwargs): - raise TransportQueryError( - "GraphQL Error", - errors=[ + raise PipefyGraphQLError( + [ { "message": "Cannot query field `broken` on type `Query`.", "extensions": {"code": "GRAPHQL_VALIDATION_FAILED"}, } - ], + ] ) executor = mock_executor(side_effect=raise_transport_error) @@ -744,11 +743,8 @@ async def mock_execute(query, variables): nonlocal call_count call_count += 1 if call_count == 1: - raise TransportQueryError( - "GraphQL Error", - errors=[ - {"message": "Cannot query field 'createCard' on type 'Query'."} - ], + raise PipefyGraphQLError( + [{"message": "Cannot query field 'createCard' on type 'Query'."}] ) # Hint lookup: return Mutation fields that include createCard return { @@ -780,9 +776,8 @@ async def mock_execute(query, variables): nonlocal call_count call_count += 1 if call_count == 1: - raise TransportQueryError( - "GraphQL Error", - errors=[{"message": "Cannot query field 'pipe' on type 'Mutation'."}], + raise PipefyGraphQLError( + [{"message": "Cannot query field 'pipe' on type 'Mutation'."}] ) return { "__type": { @@ -812,11 +807,8 @@ async def mock_execute(query, variables): nonlocal call_count call_count += 1 if call_count == 1: - raise TransportQueryError( - "GraphQL Error", - errors=[ - {"message": "Cannot query field 'nonexistent' on type 'Query'."} - ], + raise PipefyGraphQLError( + [{"message": "Cannot query field 'nonexistent' on type 'Query'."}] ) return {"__type": {"fields": [{"name": "pipe"}]}} @@ -838,11 +830,8 @@ async def mock_execute(query, variables): nonlocal call_count call_count += 1 if call_count == 1: - raise TransportQueryError( - "GraphQL Error", - errors=[ - {"message": "Cannot query field `createCard` on type `Query`."} - ], + raise PipefyGraphQLError( + [{"message": "Cannot query field `createCard` on type `Query`."}] ) return { "__type": { @@ -866,10 +855,7 @@ async def test_execute_graphql_no_hint_on_unrelated_error(): """Errors that don't match the field-not-found pattern get no hint.""" async def raise_transport_error(*_args, **_kwargs): - raise TransportQueryError( - "GraphQL Error", - errors=[{"message": "Permission denied"}], - ) + raise PipefyGraphQLError([{"message": "Permission denied"}]) executor = mock_executor(side_effect=raise_transport_error) service = SchemaIntrospectionService(executor=executor) @@ -889,14 +875,11 @@ async def mock_execute(query, variables): nonlocal call_count call_count += 1 if call_count == 1: - raise TransportQueryError( - "GraphQL Error", - errors=[ - {"message": "Cannot query field 'createCard' on type 'Query'."} - ], + raise PipefyGraphQLError( + [{"message": "Cannot query field 'createCard' on type 'Query'."}] ) # Hint lookup also fails - raise TransportQueryError("hint lookup failed", errors=[]) + raise PipefyGraphQLError([]) executor = mock_executor(side_effect=mock_execute) service = SchemaIntrospectionService(executor=executor) @@ -920,11 +903,8 @@ async def mock_execute(query, variables): nonlocal call_count call_count += 1 if call_count == 1: - raise TransportQueryError( - "GraphQL Error", - errors=[ - {"message": "Cannot query field 'createCard' on type 'Query'."} - ], + raise PipefyGraphQLError( + [{"message": "Cannot query field 'createCard' on type 'Query'."}] ) raise RuntimeError("simulated bug during hint introspection") diff --git a/packages/sdk/tests/services/test_graphql_endpoint.py b/packages/sdk/tests/services/test_graphql_endpoint.py index 9f93f6db..9255f8c5 100644 --- a/packages/sdk/tests/services/test_graphql_endpoint.py +++ b/packages/sdk/tests/services/test_graphql_endpoint.py @@ -9,7 +9,12 @@ from pipefy_auth import StaticBearerAuth from pipefy_sdk import __version__ -from pipefy_sdk.graphql_executor import GraphQLEndpoint +from pipefy_sdk.graphql_executor import ( + GraphQLEndpoint, + GraphQLResult, + PipefyGraphQLError, + data_or_raise, +) from pipefy_sdk.telemetry import telemetry_headers GRAPHQL_URL = "https://api.pipefy.com/graphql" @@ -108,7 +113,7 @@ async def test_execute_passes_variables_to_session(): request = mock_session.execute.call_args[0][0] assert isinstance(request, GraphQLRequest) assert request.variable_values == variables - assert result == {"ok": True} + assert result.data == {"ok": True} assert mock_client_cls.call_args.kwargs["fetch_schema_from_transport"] is False assert "schema" not in mock_client_cls.call_args.kwargs @@ -158,8 +163,10 @@ def _make_context_client(): mock_client_cls.side_effect = [first, second] endpoint = GraphQLEndpoint(url=GRAPHQL_URL, cache_schema=True) - assert await endpoint.execute(query, variables, auth=_bearer()) == {"one": 1} - assert await endpoint.execute(query, variables, auth=_bearer()) == {"two": 2} + first_result = await endpoint.execute(query, variables, auth=_bearer()) + second_result = await endpoint.execute(query, variables, auth=_bearer()) + assert first_result.data == {"one": 1} + assert second_result.data == {"two": 2} assert mock_client_cls.call_count == 2 assert ( @@ -196,8 +203,8 @@ async def test_execute_bubbles_up_execute_errors_unchanged(): @pytest.mark.unit @pytest.mark.asyncio -async def test_execute_query_allow_partial_returns_data_and_errors_together(): - """A response mixing data and per-node errors becomes a PartialQueryResult.""" +async def test_execute_returns_data_and_errors_together(): + """A response mixing data and per-node errors becomes a GraphQLResult.""" partial_body = { "data": {"automations": {"edges": [{"node": {"id": "25"}}]}}, "errors": [ @@ -213,9 +220,7 @@ def handler(request: httpx.Request) -> httpx.Response: with _patched_transport(handler): endpoint = GraphQLEndpoint(url=GRAPHQL_URL) - result = await endpoint.execute_allow_partial( - _sample_query(), {}, auth=_bearer() - ) + result = await endpoint.execute(_sample_query(), {}, auth=_bearer()) assert result.data == {"automations": {"edges": [{"node": {"id": "25"}}]}} assert result.errors[0]["extensions"]["automation_id"] == "124" @@ -223,17 +228,15 @@ def handler(request: httpx.Request) -> httpx.Response: @pytest.mark.unit @pytest.mark.asyncio -async def test_execute_query_allow_partial_success_carries_no_errors(): - """An error-free response yields a PartialQueryResult with an empty error list.""" +async def test_execute_success_carries_no_errors(): + """An error-free response yields a GraphQLResult with an empty error list.""" def handler(request: httpx.Request) -> httpx.Response: return httpx.Response(200, json={"data": {"automations": {"edges": []}}}) with _patched_transport(handler): endpoint = GraphQLEndpoint(url=GRAPHQL_URL) - result = await endpoint.execute_allow_partial( - _sample_query(), {}, auth=_bearer() - ) + result = await endpoint.execute(_sample_query(), {}, auth=_bearer()) assert result.data == {"automations": {"edges": []}} assert result.errors == [] @@ -241,7 +244,7 @@ def handler(request: httpx.Request) -> httpx.Response: @pytest.mark.unit @pytest.mark.asyncio -async def test_execute_query_allow_partial_null_data_yields_empty_result(): +async def test_execute_null_data_yields_empty_result(): """A fully null response yields empty data with errors kept; classifying it is the caller's job.""" def handler(request: httpx.Request) -> httpx.Response: @@ -255,14 +258,83 @@ def handler(request: httpx.Request) -> httpx.Response: with _patched_transport(handler): endpoint = GraphQLEndpoint(url=GRAPHQL_URL) - result = await endpoint.execute_allow_partial( - _sample_query(), {}, auth=_bearer() - ) + result = await endpoint.execute(_sample_query(), {}, auth=_bearer()) assert result.data == {} assert result.errors[0]["message"] == "Couldn't find Organization with 'id'=999" +@pytest.mark.unit +@pytest.mark.asyncio +async def test_execute_query_returns_data_on_success(): + """The raise-on-error convenience unwraps a clean response to its ``data`` dict.""" + + def handler(request: httpx.Request) -> httpx.Response: + return httpx.Response(200, json={"data": {"__typename": "Query"}}) + + with _patched_transport(handler): + endpoint = GraphQLEndpoint(url=GRAPHQL_URL) + result = await endpoint.execute_query(_sample_query(), {}, auth=_bearer()) + + assert result == {"__typename": "Query"} + + +@pytest.mark.unit +@pytest.mark.asyncio +async def test_execute_query_raises_pipefy_graphql_error_with_structured_errors(): + """On GraphQL errors ``execute_query`` raises ``PipefyGraphQLError`` carrying the + structured ``errors`` list; codes come off ``.errors``, not the message string.""" + error_body = { + "data": {"automations": {"edges": [{"node": {"id": "25"}}]}}, + "errors": [ + { + "message": "Permission denied", + "extensions": {"code": "PERMISSION_DENIED", "automation_id": "124"}, + } + ], + } + + def handler(request: httpx.Request) -> httpx.Response: + return httpx.Response(200, json=error_body) + + with _patched_transport(handler): + endpoint = GraphQLEndpoint(url=GRAPHQL_URL) + with pytest.raises(PipefyGraphQLError) as excinfo: + await endpoint.execute_query(_sample_query(), {}, auth=_bearer()) + + assert excinfo.value.errors[0]["message"] == "Permission denied" + assert excinfo.value.errors[0]["extensions"]["code"] == "PERMISSION_DENIED" + assert str(excinfo.value) == "Permission denied" + + +@pytest.mark.unit +def test_data_or_raise_returns_data_on_clean_result(): + """A result with no errors passes its ``data`` through unchanged.""" + result = GraphQLResult(data={"pipe": {"id": "1"}}, errors=[]) + assert data_or_raise(result) == {"pipe": {"id": "1"}} + + +@pytest.mark.unit +def test_pipefy_graphql_error_tolerates_non_dict_error_item(): + """A non-conforming error element that is not a dict does not crash the message.""" + exc = PipefyGraphQLError(["a bare string", {"message": "a real message"}]) + assert "a bare string" in str(exc) + assert "a real message" in str(exc) + + +@pytest.mark.unit +def test_data_or_raise_raises_pipefy_graphql_error_on_errors(): + """A result carrying errors raises ``PipefyGraphQLError`` with those errors.""" + errors = [ + {"message": "Permission denied", "extensions": {"code": "PERMISSION_DENIED"}} + ] + result = GraphQLResult(data={}, errors=errors) + with pytest.raises(PipefyGraphQLError) as excinfo: + data_or_raise(result) + assert excinfo.value.errors == errors + assert str(excinfo.value) == "Permission denied" + + @pytest.mark.unit def test_init_connects_to_given_url(): """The endpoint connects to exactly the URL it is handed, no derivation.""" diff --git a/packages/sdk/tests/services/test_internal_api_executor.py b/packages/sdk/tests/services/test_internal_api_executor.py index d016d747..18384894 100644 --- a/packages/sdk/tests/services/test_internal_api_executor.py +++ b/packages/sdk/tests/services/test_internal_api_executor.py @@ -1,9 +1,8 @@ """Unit tests for the Internal API executor and PipefySettings.internal_api_url. -These tests intentionally assert the full GraphQL error text produced by the -Internal API executor (the ``HttpxGraphQLExecutor`` that ``build_executors`` -aims at ``internal_api_url`` with the ``[code=...]`` / ``[correlation_id=...]`` -envelope formatter). +The internal endpoint raises ``PipefyGraphQLError`` on GraphQL errors, carrying +the raw ``errors`` list; code and correlation_id come off each error's +``extensions`` rather than a formatted message string. """ import json @@ -16,7 +15,7 @@ from pipefy_auth import StaticBearerAuth from pipefy_sdk.client import build_executors -from pipefy_sdk.graphql_executor import GraphQLExecutor +from pipefy_sdk.graphql_executor import GraphQLExecutor, PipefyGraphQLError from pipefy_sdk.settings import PipefySettings DEFAULT_INTERNAL_API_URL = "https://app.pipefy.com/internal_api" @@ -104,7 +103,7 @@ async def test_execute_query_raises_on_graphql_errors_in_body(respx_mock): return_value=httpx.Response(200, json=graphql_error_response) ) - with pytest.raises(ValueError, match=r"^Something went wrong$"): + with pytest.raises(PipefyGraphQLError, match=r"^Something went wrong$"): await _build_executor().execute_query(gql("query { x }"), {}) @@ -114,7 +113,7 @@ async def test_execute_query_raises_on_graphql_errors_in_body(respx_mock): async def test_execute_query_error_includes_extensions_code_and_correlation_id( respx_mock, ): - """GraphQL error message includes extensions code and correlation_id when present.""" + """The raised error carries extensions code and correlation_id in its structured errors.""" graphql_error_response = { "errors": [ { @@ -130,12 +129,14 @@ async def test_execute_query_error_includes_extensions_code_and_correlation_id( return_value=httpx.Response(200, json=graphql_error_response) ) - with pytest.raises( - ValueError, - match=r"Permission Denied \[code=PERMISSION_DENIED\] \[correlation_id=abc-123\]", - ): + with pytest.raises(PipefyGraphQLError) as excinfo: await _build_executor().execute_query(gql("query { x }"), {}) + extensions = excinfo.value.errors[0]["extensions"] + assert extensions["code"] == "PERMISSION_DENIED" + assert extensions["correlation_id"] == "abc-123" + assert str(excinfo.value) == "Permission Denied" + @pytest.mark.unit @pytest.mark.asyncio @@ -143,7 +144,7 @@ async def test_execute_query_error_includes_extensions_code_and_correlation_id( async def test_execute_query_error_includes_correlation_id_when_code_absent( respx_mock, ): - """``correlation_id`` is appended even when ``extensions.code`` is missing.""" + """``correlation_id`` is carried in the structured errors even without ``extensions.code``.""" graphql_error_response = { "errors": [ { @@ -156,18 +157,20 @@ async def test_execute_query_error_includes_correlation_id_when_code_absent( return_value=httpx.Response(200, json=graphql_error_response) ) - with pytest.raises( - ValueError, - match=r"^Rate limited \[correlation_id=corr-only-99\]$", - ): + with pytest.raises(PipefyGraphQLError) as excinfo: await _build_executor().execute_query(gql("query { x }"), {}) + extensions = excinfo.value.errors[0]["extensions"] + assert "code" not in extensions + assert extensions["correlation_id"] == "corr-only-99" + assert str(excinfo.value) == "Rate limited" + @pytest.mark.unit @pytest.mark.asyncio @respx.mock(assert_all_mocked=False, assert_all_called=False) async def test_execute_query_error_concatenates_multiple_errors(respx_mock): - """Multiple GraphQL errors are joined with '; ' in the raised ValueError message.""" + """Multiple GraphQL error messages are joined with '; ' in the raised message.""" graphql_error_response = { "errors": [ {"message": "Error one"}, @@ -178,18 +181,22 @@ async def test_execute_query_error_concatenates_multiple_errors(respx_mock): return_value=httpx.Response(200, json=graphql_error_response) ) - with pytest.raises( - ValueError, - match=r"^Error one; Error two \[code=BAD_INPUT\]$", - ): + with pytest.raises(PipefyGraphQLError) as excinfo: await _build_executor().execute_query(gql("query { x }"), {}) + assert str(excinfo.value) == "Error one; Error two" + assert [e.get("message") for e in excinfo.value.errors] == [ + "Error one", + "Error two", + ] + assert excinfo.value.errors[1]["extensions"]["code"] == "BAD_INPUT" + @pytest.mark.unit @pytest.mark.asyncio @respx.mock(assert_all_mocked=False, assert_all_called=False) async def test_execute_query_error_without_message_uses_fallback(respx_mock): - """GraphQL error dict without message uses 'Unknown error' as the base text.""" + """GraphQL error dict without message uses 'Unknown error' as the joined text.""" graphql_error_response = { "errors": [{"extensions": {"code": "UNKNOWN"}}], } @@ -197,12 +204,12 @@ async def test_execute_query_error_without_message_uses_fallback(respx_mock): return_value=httpx.Response(200, json=graphql_error_response) ) - with pytest.raises( - ValueError, - match=r"^Unknown error \[code=UNKNOWN\]$", - ): + with pytest.raises(PipefyGraphQLError) as excinfo: await _build_executor().execute_query(gql("query { x }"), {}) + assert str(excinfo.value) == "Unknown error" + assert excinfo.value.errors[0]["extensions"]["code"] == "UNKNOWN" + @pytest.mark.unit @pytest.mark.asyncio diff --git a/packages/sdk/tests/services/test_pipefy_facade.py b/packages/sdk/tests/services/test_pipefy_facade.py index 9ee91f1b..b3752f98 100644 --- a/packages/sdk/tests/services/test_pipefy_facade.py +++ b/packages/sdk/tests/services/test_pipefy_facade.py @@ -5,6 +5,7 @@ from pipefy_sdk import __version__ from pipefy_sdk.client import PipefyClient, build_executors +from pipefy_sdk.graphql_executor import GraphQLResult from pipefy_sdk.services.ai_agent_service import AiAgentService from pipefy_sdk.services.attachment_service import AttachmentService from pipefy_sdk.services.automation_service import AutomationService @@ -790,7 +791,9 @@ async def test_delete_card_relation_delegates_to_internal_api_client(mock_settin # delegates to its shared endpoint; swap that endpoint's network seam. internal = client._internal_executor internal.endpoint.execute = AsyncMock( - return_value={"deleteCardRelation": {"success": True}} + return_value=GraphQLResult( + data={"deleteCardRelation": {"success": True}}, errors=[] + ) ) # Pin the snake_case input keys that the Internal API expects @@ -818,7 +821,9 @@ async def test_sub_portal_mutation_routes_through_internal_api_client(mock_setti # PortalService and the facade share the one internal executor and its endpoint. internal = client._internal_executor internal.endpoint.execute = AsyncMock( - return_value={"updateSubPortalElement": {"success": True}} + return_value=GraphQLResult( + data={"updateSubPortalElement": {"success": True}}, errors=[] + ) ) result = await client.publish_sub_portal("portal-1", "element-2", "sub-3")