Skip to content

Fix short circuit introspection for namespace like fields - #697

Merged
gnawf merged 2 commits into
masterfrom
fix-namespace-like-short-circuit-introspection
Aug 11, 2025
Merged

Fix short circuit introspection for namespace like fields#697
gnawf merged 2 commits into
masterfrom
fix-namespace-like-short-circuit-introspection

Conversation

@gnawf

@gnawf gnawf commented Aug 11, 2025

Copy link
Copy Markdown
Collaborator

No description provided.

@github-actions

github-actions Bot commented Aug 11, 2025

Copy link
Copy Markdown

Test Results

  605 files  +  605    605 suites  +605   1m 5s ⏱️ + 1m 5s
2 006 tests +2 006  1 491 ✅ +1 491  515 💤 +515  0 ❌ ±0 
2 014 runs  +2 014  1 499 ✅ +1 499  515 💤 +515  0 ❌ ±0 

Results for commit 6d3c609. ± Comparison against base commit f86b8ce.

♻️ This comment has been updated with latest results.

}
val operationType = service.underlyingSchema.getTypeAs<GraphQLObjectType>(topLevelField.singleObjectTypeName)
val topLevelFieldDefinition = operationType.getField(topLevelField.name)
val isNamespacedLike = topLevelFieldDefinition?.arguments?.isEmpty() == true

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extracted this code into common NamespacedUtil.isNamespaceFieldLike

}
val operationType = service.underlyingSchema.getTypeAs<GraphQLObjectType>(topLevelField.singleObjectTypeName)
val topLevelFieldDefinition = operationType.getField(topLevelField.name)
val isNamespacedLike = topLevelFieldDefinition?.arguments?.isEmpty() == true

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So the bug here is that the logic we use to identify "namespaced fields" is different here and in the internal introspection service.

This leads to a bug because this code thinks more fields can be handled by the internal introspection service, but in reality it can't so it fails.

getFieldsWithCoordinates(schema.queryType, schema.mutationType, schema.subscriptionType)
.filter { (_, field) ->
isNamespacedField(field)
isNamespacedFieldLike(field)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this uses the newly extracted code from NextgenEngine.isOnlyTopLevelFieldTypename

@Suppress("unused")
public class NamespaceLikeShortCircuitsIntrospectionTestSnapshot : TestSnapshot() {
override val calls: List<ExpectedServiceCall> = listOf(
)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So here the calls are empty because the tests are executed by our internal introspection service.

| {
| "data": {
| "test": {
| "__typename": "TestQuery"

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Prior to this PR's fix, it would error out like so

{
  "errors": [
    {
      "message": "Exception while fetching data (/test) : Internal error: should never happen: This data fetcher should NEVER be called from Nadel",
      "locations": [
        {
          "column": 2,
          "line": 1
        }
      ],
      "path": [
        "test"
      ],
      "extensions": {
        "classification": "DataFetchingException"
      }
    }
  ],
  "data": {
    "test": null
  }
}

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding some stubbed tests because that's what led to this bug.

@gnawf
gnawf merged commit 039d318 into master Aug 11, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants