Skip to content

fix: return empty list for nullable lists#1225

Merged
Noroth merged 2 commits intomasterfrom
ludwig/fix-null-lists
Jul 11, 2025
Merged

fix: return empty list for nullable lists#1225
Noroth merged 2 commits intomasterfrom
ludwig/fix-null-lists

Conversation

@Noroth
Copy link
Copy Markdown
Contributor

@Noroth Noroth commented Jul 11, 2025

We currently do not yet support non nullable lists therefore we need to return an empty list for now in order to be able to work with both on GraphQL side.

Summary by CodeRabbit

  • Bug Fixes
    • List fields in gRPC datasource responses now consistently return empty arrays instead of null when invalid, ensuring more predictable JSON output.

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.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Jul 11, 2025

Walkthrough

The update revises the marshalResponseJSON method in the gRPC datasource to always set list fields as empty JSON arrays when invalid, instead of setting them to null. The distinction between nullable and non-nullable lists is removed, and a comment is added about future support for nullable lists.

Changes

File(s) Change Summary
v2/pkg/engine/datasource/grpc_datasource/grpc_datasource.go Updated list field handling in marshalResponseJSON to always use empty arrays; added a comment.

📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 23a6024 and 5b74f30.

📒 Files selected for processing (1)
  • v2/pkg/engine/datasource/grpc_datasource/grpc_datasource.go (1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: SkArchon
PR: wundergraph/graphql-go-tools#1203
File: v2/pkg/engine/resolve/loader.go:63-67
Timestamp: 2025-07-02T15:28:02.122Z
Learning: In the graphql-go-tools codebase, result structs are consistently initialized with non-nil bytes.Buffer instances, making additional nil checks for res.out unnecessary defensive programming.
v2/pkg/engine/datasource/grpc_datasource/grpc_datasource.go (1)
Learnt from: SkArchon
PR: wundergraph/graphql-go-tools#1203
File: v2/pkg/engine/resolve/loader.go:63-67
Timestamp: 2025-07-02T15:28:02.122Z
Learning: In the graphql-go-tools codebase, result structs are consistently initialized with non-nil bytes.Buffer instances, making additional nil checks for res.out unnecessary defensive programming.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: Build and test (go 1.23 / ubuntu-latest)
  • GitHub Check: Build and test (go 1.23 / windows-latest)
  • GitHub Check: Build and test (go 1.23 / windows-latest)
🔇 Additional comments (1)
v2/pkg/engine/datasource/grpc_datasource/grpc_datasource.go (1)

187-192: Ensure GraphQL list nullability semantics for nullable vs non-nullable fields

This temporary fix in
v2/pkg/engine/datasource/grpc_datasource/grpc_datasource.go (lines 187–192)
always returns an empty array (arena.NewArray()) for all list fields, effectively treating every list as non-nullable. Given your GraphQL schemas define both nullable ([Type]) and non-nullable ([Type]! or [Type!]!) lists, please:

• Verify that no clients rely on receiving null for fields declared as nullable lists in your .graphql/.gql schema files (e.g. pkg/codegen/testdata/schema.graphql, pkg/testing/subscriptiontesting/schema.graphql, etc.).
• Add or update unit tests around marshalResponseJSON to cover both nullable and non-nullable list scenarios, asserting empty arrays vs null as appropriate.
• Confirm this temporary behavior aligns with your current schema compliance goals and document any client-facing breaking changes.

Once schema compatibility and test coverage are in place, this workaround can be approved pending the full non-nullable/nullable list distinction implementation.

✨ Finishing Touches
  • 📝 Generate Docstrings

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Copy Markdown
Collaborator

@StarpTech StarpTech left a comment

Choose a reason for hiding this comment

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

LGTM

@Noroth Noroth merged commit 1166a10 into master Jul 11, 2025
11 checks passed
@Noroth Noroth deleted the ludwig/fix-null-lists branch July 11, 2025 07:37
Noroth pushed a commit that referenced this pull request Jul 14, 2025
🤖 I have created a release *beep* *boop*
---


##
[2.0.0-rc.205](v2.0.0-rc.204...v2.0.0-rc.205)
(2025-07-14)


### Bug Fixes

* missing field refs in union types
([#1228](#1228))
([57e1d38](57e1d38))
* return empty list for nullable lists
([#1225](#1225))
([1166a10](1166a10))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **Bug Fixes**
  * Resolved missing field references in union types.
* Ensured nullable lists now return an empty list instead of null or
other values.

* **Documentation**
* Added a changelog entry for version 2.0.0-rc.205 detailing recent bug
fixes.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
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