fix: sanitize git GraphQL query import errors#9815
Conversation
|
Hi @fatih-acar and @saltas888, I opened this small PR after reproducing the noisy Git GraphQL query import error locally and matching it to #7498. The change is intentionally narrow: it catches the SDK GraphQLError in the Git integration query-creation path, keeps the repository/query context, and strips the raw generated mutation/variables from the operator-facing task message. I also added regression coverage and verified the behavior in a local InfraHub task UI run. Would appreciate your review when you have a chance. |
ogenstad
left a comment
ogenstad
left a comment
There was a problem hiding this comment.
In Infrahub 1.10.0 we introduced an error catalogue: https://github.com/opsmill/infrahub/blob/infrahub-v1.10.2/schema/error-catalogue.json
These errors will be added in the error extension field of the GraphQL errors, the idea is that the catalogue will grow over time and that we'll use this json file to generate error bindings that the SDK then uses so that we get the enriched errors directly within the SDK. (some more information about this can be found here: https://github.com/opsmill/infrahub/tree/infrahub-v1.10.2/dev/specs/infp-468-graphql-error-catalogue)
As such I think the next step for this should be to introduce this parsing within the SDK and ensure that the backend raises the correct enriched error instead of having a method within the integrator that tries to parse the error again with something like the _clean_sdk_graphql_error_message() function.
| import shutil | ||
| from pathlib import Path | ||
| from unittest.mock import AsyncMock, patch | ||
| from unittest.mock import AsyncMock, MagicMock, patch |
There was a problem hiding this comment.
We don't want to use MagicMock for anything new, the goal is to remove any MagicMock or AsyncMock.
|
Thanks @ogenstad, agreed. I ran this locally and the better fix is to move it down a layer instead of parsing in the integrator. I’ll update the PR to add a I’ll keep broader validation/catalogue cases out of this PR unless you’d prefer those included too. |
… queries [opsmill#7498] Rework the git import error handling per review feedback: - Add a GRAPHQL_QUERY_INVALID catalogue entry (422, evolving), raised as GraphQLQueryInvalidError from the GraphQLQuery create/update/upsert mutations with the concise graphql-core validation messages, replacing ValueError("Query is not valid, [GraphQLError(...)]"). - Regenerate schema/error-catalogue.json, the frontend error bindings and the error-catalogue reference docs. - Remove the regex-based message re-parsing from the git integrator: both the create and update import paths now wrap the server-provided error messages in RepositoryConfigurationError and suppress the chained SDK exception, which embeds the full rendered mutation. - Replace the MagicMock/AsyncMock regression test with a pytest-httpx test driving the real SDK client, and add mutation-level tests asserting the catalogued error extensions for create and update. A follow-up in opsmill/infrahub-sdk-python will parse the error catalogue extensions so SDK consumers get the enriched error without message joining.
Done in 48ba4c2. The backend now raises a catalogued The SDK parsing is up in opsmill/infrahub-sdk-python#1140. |
The leak checks on caplog were only meaningful if log capture worked at all; a positive assertion on the sanitized message now guards them against passing vacuously.
4 participants
Problem
Git repository imports can fail while creating
CoreGraphQLQuerynodes. When that happens, the SDKGraphQLErrorcurrently surfaces the full generated mutation, variables, location reprs, and traceback-heavy text in task logs. That makes a user/configuration error hard to scan and can expose implementation details that are not useful to operators.Change
infrahub_sdk.exceptions.GraphQLErroraround GraphQL query creation during Git importRepositoryConfigurationErrorthat keeps the repository name, query name, and server validation messageSourceLocationreprs from the operator-facing error/log messageBefore / After
Before, the worker/task output could include the full
CoreGraphQLQueryCreatemutation and SDK exception payload.After, the task reports a focused message like:
Screenshot
Filtered task log showing the sanitized
RepositoryConfigurationErrorafter reproducing the Git import failure locally:Scope
This targets the Git integration path from #7498. It is related to the broader user-error logging cleanup in #4589, but intentionally does not attempt to change general API/GraphQL resolver logging in this PR.
Testing
uv run ruff check backend/infrahub/git/integrator.py backend/tests/component/git/test_graphql_query_import.pyuv run pytest backend/tests/component/git/test_graphql_query_import.py -qAddresses #7498. Related to #4589.