-
Notifications
You must be signed in to change notification settings - Fork 7
[CDF-28123] fix(graphql): surface real API error when upsertGraphQlDmlVersion returns null #3088
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
a68418b
b67b1a1
a39ab3a
892a565
806464e
fbe0a5f
f1a33bb
9a67e65
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -3,6 +3,7 @@ | |||||||||||||||||||
| This API provides a wrapper around the legacy DML API for managing GraphQL data models. | ||||||||||||||||||||
| """ | ||||||||||||||||||||
|
|
||||||||||||||||||||
| import json | ||||||||||||||||||||
| from collections.abc import Iterable, Sequence | ||||||||||||||||||||
| from typing import Any | ||||||||||||||||||||
|
|
||||||||||||||||||||
|
|
@@ -26,13 +27,22 @@ | |||||||||||||||||||
| from cognite_toolkit._cdf_tk.utils import humanize_collection | ||||||||||||||||||||
|
|
||||||||||||||||||||
|
|
||||||||||||||||||||
| class DMLError(BaseModel): | ||||||||||||||||||||
| model_config = ConfigDict(extra="allow") | ||||||||||||||||||||
| kind: str | None = None | ||||||||||||||||||||
| message: str | None = None | ||||||||||||||||||||
| hint: str | None = None | ||||||||||||||||||||
|
|
||||||||||||||||||||
|
|
||||||||||||||||||||
| class UpsertResponseData(BaseModel): | ||||||||||||||||||||
| errors: dict[str, Any] | None = None | ||||||||||||||||||||
| result: GraphQLDataModelResponse | ||||||||||||||||||||
| errors: list[DMLError] | None = None | ||||||||||||||||||||
| result: GraphQLDataModelResponse | None = None | ||||||||||||||||||||
|
|
||||||||||||||||||||
|
|
||||||||||||||||||||
| class GraphQLUpsertResponse(BaseModel): | ||||||||||||||||||||
| upsert_graph_ql_dml_version: UpsertResponseData = Field(alias="upsertGraphQlDmlVersion") | ||||||||||||||||||||
| # Nullable: the API sets this to null and populates top-level errors when | ||||||||||||||||||||
| # the mutation input is rejected (e.g. unknown fields, auth failures). | ||||||||||||||||||||
| upsert_graph_ql_dml_version: UpsertResponseData | None = Field(None, alias="upsertGraphQlDmlVersion") | ||||||||||||||||||||
|
|
||||||||||||||||||||
|
|
||||||||||||||||||||
| class GraphQLErrors(BaseModel): | ||||||||||||||||||||
|
|
@@ -43,7 +53,7 @@ class GraphQLErrors(BaseModel): | |||||||||||||||||||
|
|
||||||||||||||||||||
|
|
||||||||||||||||||||
| class GraphQLResponse(BaseModel): | ||||||||||||||||||||
| data: GraphQLUpsertResponse | ||||||||||||||||||||
| data: GraphQLUpsertResponse | None = None | ||||||||||||||||||||
| errors: list[GraphQLErrors] | None = None | ||||||||||||||||||||
|
|
||||||||||||||||||||
|
|
||||||||||||||||||||
|
|
@@ -79,11 +89,21 @@ def _post_graphql(self, payload: dict[str, Any]) -> GraphQLUpsertResponse: | |||||||||||||||||||
| ) | ||||||||||||||||||||
| result = self._http_client.request_single_retries(request) | ||||||||||||||||||||
| response = result.get_success_or_raise(request) | ||||||||||||||||||||
| parsed = GraphQLResponse.model_validate_json(response.body) | ||||||||||||||||||||
| if errors := parsed.errors: | ||||||||||||||||||||
| raise ToolkitAPIError( | ||||||||||||||||||||
| f"Failed GraphQL errors: {humanize_collection([error.message for error in errors if error.message])}" | ||||||||||||||||||||
| ) | ||||||||||||||||||||
| # Parse as raw dict first so top-level GraphQL errors (which accompany a | ||||||||||||||||||||
| # null upsertGraphQlDmlVersion) are surfaced before Pydantic validation. | ||||||||||||||||||||
| raw = json.loads(response.body) | ||||||||||||||||||||
| if top_errors := raw.get("errors"): | ||||||||||||||||||||
| messages = [e.get("message", str(e)) for e in top_errors if isinstance(e, dict)] | ||||||||||||||||||||
| raise ToolkitAPIError(f"GraphQL mutation failed: {humanize_collection(messages)}") | ||||||||||||||||||||
| parsed = GraphQLResponse.model_validate(raw) | ||||||||||||||||||||
| if parsed.data is None: | ||||||||||||||||||||
| raise ToolkitAPIError("GraphQL mutation returned no data and no errors.") | ||||||||||||||||||||
| upsert = parsed.data.upsert_graph_ql_dml_version | ||||||||||||||||||||
| if upsert is None: | ||||||||||||||||||||
| raise ToolkitAPIError("GraphQL mutation returned no result and no errors.") | ||||||||||||||||||||
| if upsert.errors: | ||||||||||||||||||||
| messages = [e.message for e in upsert.errors if e.message] | ||||||||||||||||||||
| raise ToolkitAPIError(f"DML validation failed: {humanize_collection(messages)}") | ||||||||||||||||||||
|
Comment on lines
+104
to
+106
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If
Suggested change
References
|
||||||||||||||||||||
| return parsed.data | ||||||||||||||||||||
|
|
||||||||||||||||||||
| def create(self, items: Sequence[GraphQLDataModelRequest]) -> list[GraphQLDataModelResponse]: | ||||||||||||||||||||
|
|
@@ -102,7 +122,11 @@ def create(self, items: Sequence[GraphQLDataModelRequest]) -> list[GraphQLDataMo | |||||||||||||||||||
| "variables": {"dmCreate": item.model_dump(mode="json", by_alias=True, exclude_unset=False)}, | ||||||||||||||||||||
| } | ||||||||||||||||||||
| response = self._post_graphql(payload) | ||||||||||||||||||||
| results.append(response.upsert_graph_ql_dml_version.result) | ||||||||||||||||||||
| # _post_graphql raises before returning if upsert_graph_ql_dml_version or result is None. | ||||||||||||||||||||
| upsert = response.upsert_graph_ql_dml_version | ||||||||||||||||||||
| if upsert is None or upsert.result is None: | ||||||||||||||||||||
| raise ToolkitAPIError("GraphQL mutation succeeded but returned no data model.") | ||||||||||||||||||||
| results.append(upsert.result) | ||||||||||||||||||||
| return results | ||||||||||||||||||||
|
|
||||||||||||||||||||
| def retrieve(self, items: Sequence[DataModelId], inline_views: bool = False) -> list[GraphQLDataModelResponse]: | ||||||||||||||||||||
|
|
||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the response body is not a JSON object (e.g., if it is a JSON array, a primitive, or if an intermediate proxy returns an unexpected format),
rawwill not be a dictionary and callingraw.getwill raise anAttributeError. Additionally, iftop_errorsis not a list or contains non-dictionary elements, the current list comprehension will either ignore them or fail. Checking thatrawis a dictionary and handling non-list/non-dict errors gracefully ensures robust error surfacing.References