fix: entity null fetch produces resolver error

[dkorittki]

Checklist

• I have discussed my proposed changes in an issue and have received approval to proceed.
• I have followed the coding standards of the project.
• Tests or benchmarks have been added or updated.

Context

When the engine receives responses from subgraph entity fetches during resolving it currently does not differentiate between null data responses { "data": null } and null entity responses { "data": { "_entities": [ null ] } }.
In both cases the engine adds a no data or errors in response resolver error but this is not correct. In case of a null entity fetch the subgraph returned actual data: an array containing null elements. Instead of creating an error it should simply skip merging it into the result.

Summary by CodeRabbit

• Bug Fixes

  • Improved handling of null/empty entity fetches so empty fetches are skipped and unrelated subgraph errors are not attached; non-nullable root fields still produce errors while nullable fields preserve nulls.

• Tests

  • Added comprehensive tests covering single and batch null-entity scenarios to validate error reporting, null propagation, and control flow.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

Adds a pre-merge guard in mergeResult to skip merging when a subgraph returns null for an entity or entity-batch fetch via data._entities. Introduces isEmptyEntityFetch(fetchItem *FetchItem, response *astjson.Value) bool and extends resolver tests covering null-entity single/batch and nullable vs non-nullable roots.

Changes

Cohort / File(s)	Summary
Loader: null-entity guard v2/pkg/engine/resolve/loader.go	Adds an early-return in mergeResult when a post-processing path exists and the subgraph response represents an empty entity/entity-batch fetch. Adds private helper isEmptyEntityFetch(fetchItem *FetchItem, response *astjson.Value) bool.
Tests: null-entity scenarios v2/pkg/engine/resolve/resolve_test.go	Adds multiple subtests to TestResolver_ResolveGraphQLResponse exercising subgraphs returning {"data":{"_entities":[null]}} and {"data":null} for single and batch entity fetches; covers nullable vs non-nullable root-field behavior and error propagation.

Sequence Diagram(s)

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• feat: validate presence of optional @requires dependencies #1297: Overlaps on handling null entries in data._entities and related merge/error behavior in loader.go.
• fix: improve handling for invalid responses #1283: Related changes to mergeResult affecting batch merge and validation logic in loader.go.

🚥 Pre-merge checks | ✅ 2

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'fix: entity null fetch produces resolver error' directly summarizes the main change: preventing null entity fetch responses from producing unwanted resolver errors.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch dominik/eng-7492-fix-data-entities-null-produces-fetch-error-no-data-or

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[dkorittki]

force pushed because I rebased this pr on top of main branch

[coderabbitai]

Actionable comments posted: 1

ℹ️ Review info

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2df3968 and 8542fe4.

📒 Files selected for processing (1)

• v2/pkg/engine/resolve/loader.go

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@v2/pkg/engine/resolve/loader.go`:
- Around line 642-643: Fix the typo in the doc comment for isEmptyEntityFetch:
change "sucessful" to "successful" in the comment above the isEmptyEntityFetch
function so the documentation reads "successful entity fetch".

[coderabbitai]

🧹 Nitpick comments (1)

v2/pkg/engine/resolve/resolve_test.go (1)

2411-2482: Consider adding the missing batch+nullable root variant for symmetry.

You covered batch null entity with a non-nullable root field, but the nullable-root equivalent is still untested in this block. Adding it would complete the behavior matrix and reduce regression risk.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@v2/pkg/engine/resolve/resolve_test.go` around lines 2411 - 2482, Add a
parallel test case to cover the "batch null entity with nullable root field"
variant: copy the existing t.Run("fetch with null entity batch and non-nullable
root field", ...) case but set the root Data.Nullable = true (the
GraphQLResponse.Fetches SingleWithPath BatchEntityFetch -> Data Nullable field)
and adjust the expectedOutput to the nullable-root expectation (e.g. no
non-nullable error and the field value is null in the data JSON). Keep the same
mock DataSource behavior (NewMockDataSource.Load returning
{"data":{"_entities":[null]}}) and the same BatchEntityFetch config
(SkipNullItems/SkipErrItems, SelectResponseDataPath, etc.) to verify the
resolver returns a null field rather than an error when the root is nullable.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@v2/pkg/engine/resolve/resolve_test.go`:
- Around line 2411-2482: Add a parallel test case to cover the "batch null
entity with nullable root field" variant: copy the existing t.Run("fetch with
null entity batch and non-nullable root field", ...) case but set the root
Data.Nullable = true (the GraphQLResponse.Fetches SingleWithPath
BatchEntityFetch -> Data Nullable field) and adjust the expectedOutput to the
nullable-root expectation (e.g. no non-nullable error and the field value is
null in the data JSON). Keep the same mock DataSource behavior
(NewMockDataSource.Load returning {"data":{"_entities":[null]}}) and the same
BatchEntityFetch config (SkipNullItems/SkipErrItems, SelectResponseDataPath,
etc.) to verify the resolver returns a null field rather than an error when the
root is nullable.

---

ℹ️ Review info

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8542fe4 and 383faee.

📒 Files selected for processing (2)

• v2/pkg/engine/resolve/loader.go
• v2/pkg/engine/resolve/resolve_test.go

🚧 Files skipped from review as they are similar to previous changes (1)

• v2/pkg/engine/resolve/loader.go

[ryou90]

Is there any chance this fix will be released soon?

[dkorittki]

Is there any chance this fix will be released soon?

Yes, working on it