Introduces MakeFieldsNullableTypeInterceptor to prevent null propagation#175
Introduces MakeFieldsNullableTypeInterceptor to prevent null propagation#175MK-simplic wants to merge 1 commit into
Conversation
WalkthroughThe pull request extends the Changes
Sequence DiagramsequenceDiagram
participant App as Application
participant Svc as IServiceCollection
participant REB as IRequestExecutorBuilder
participant HC as HotChocolate Type System
participant Interc as MakeFieldsNullableTypeInterceptor
App->>Svc: UseGraphQL<TQuery>(tolerateMissingFieldValues: true)
Svc->>REB: builder configuration
alt tolerateMissingFieldValues is true
REB->>Interc: TryAddTypeInterceptor()
end
Svc->>HC: Build schema
HC->>Interc: OnBeforeCompleteName()
loop For each field in ObjectType
Interc->>Interc: Check if field requires nullability
Interc->>Interc: ChangeNullability to true
Interc->>HC: Update field type
end
HC-->>App: Schema with nullable fields
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
GraphQL Nullable Fields InterceptorThe Problem: Null PropagationIf a document in MongoDB is missing the Per GraphQL spec, when a resolver returns The Solution:
|
| Without Interceptor (Strict) | With Interceptor (Tolerant) |
|---|---|
type Product { |
type Product { |
sku: String! |
sku: String |
price: Decimal! |
price: Decimal |
} |
} |
3. JSON Response Example
Given a document where sku is missing:
Without interceptor (strict, spec-conformant):
{
"data": { "product": null },
"errors": [
{
"message": "Cannot return null for non-nullable field Product.sku.",
"path": ["product", "sku"]
}
]
}With interceptor (tolerant):
{
"data": {
"product": {
"sku": null,
"price": 12.50
}
}
}Benefits
- Data resilience — prevents one missing field from breaking the entire UI component.
- Legacy compatibility — seamlessly handles MongoDB documents that were saved before certain fields were added to the C# models.
- Developer velocity — no need to manually mark every C# property as nullable just to satisfy the GraphQL schema.
Integration
The interceptor is registered by default through the shared Simplic.OxS.Server extension:
services.UseGraphQL<Query>(); // tolerant (default)
services.UseGraphQL<Query>(tolerateMissingFieldValues: false); // strict, spec-conformantWhen enabled, MakeFieldsNullableTypeInterceptor is registered via TryAddTypeInterceptor, so every service that consumes Simplic.OxS.Server benefits automatically — no per-service startup change needed.
Caveats
- Schema becomes broadly nullable. Frontend/codegen clients must expect fields that were previously
String!to now beString. Strongly typed codegens (gql-codegen, Strawberry Shake) will regenerate — calling code may neednull-checks it didn't need before. - Framework fields are preserved. The interceptor skips types whose namespace starts with
HotChocolate, so paging (Connection,Edge,PageInfo) and introspection remain strict. - Opt-out per service. Pass
tolerateMissingFieldValues: falsetoUseGraphQL<TQuery>if a specific service needs strict non-null guarantees.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/Simplic.OxS.Server/Extensions/GraphQLExtension.cs (1)
25-37:⚠️ Potential issue | 🟠 MajorDefaulting tolerance to
trueintroduces a silent schema-contract shift.Line 25 makes existing
UseGraphQL<TQuery>()call sites automatically emit looser nullability, and Line 36 applies it globally. This is high-impact for schema consumers/tooling (including exported schema snapshots). Consider defaulting tofalseand requiring explicit opt-in.Suggested fix
- bool tolerateMissingFieldValues = true) where TQuery : class + bool tolerateMissingFieldValues = false) where TQuery : class🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Simplic.OxS.Server/Extensions/GraphQLExtension.cs` around lines 25 - 37, The method UseGraphQL<TQuery>() currently defaults tolerateMissingFieldValues to true causing an implicit global nullability relaxation; change the parameter default to false (tolerateMissingFieldValues = false) so callers must opt-in, keep the existing conditional that adds MakeFieldsNullableTypeInterceptor only when tolerateMissingFieldValues is true, and update any related tests/docs/examples that assumed the previous default to explicitly pass true where needed; reference the UseGraphQL<TQuery>() signature, the tolerateMissingFieldValues parameter, and the MakeFieldsNullableTypeInterceptor addition in your changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/Simplic.OxS.Server/Extensions/GraphQLExtension.cs`:
- Around line 24-25: The method parameter 'builder' in GraphQLExtension.cs is
being assigned null but is declared non-nullable; change its type to a nullable
delegate by updating the parameter declaration from
Action<IRequestExecutorBuilder> builder = null to
Action<IRequestExecutorBuilder>? builder = null (i.e., make the 'builder'
parameter nullable) so it aligns with nullable reference types and the method
signature (where TQuery : class).
In `@src/Simplic.OxS.Server/GraphQL/MakeFieldsNullableTypeInterceptor.cs`:
- Around line 35-37: The namespace check in MakeFieldsNullableTypeInterceptor is
too broad: the runtimeType?.Namespace StartsWith("HotChocolate", ...) will also
match names like "HotChocolateExtensions" and skip user types; change the guard
to only skip the exact HotChocolate root or its subnamespaces by checking either
Namespace.Equals("HotChocolate", StringComparison.Ordinal) ||
Namespace.StartsWith("HotChocolate.", StringComparison.Ordinal) (or the
equivalent) so runtimeType.Namespace only bypasses the interceptor for
"HotChocolate" and "HotChocolate.*".
---
Outside diff comments:
In `@src/Simplic.OxS.Server/Extensions/GraphQLExtension.cs`:
- Around line 25-37: The method UseGraphQL<TQuery>() currently defaults
tolerateMissingFieldValues to true causing an implicit global nullability
relaxation; change the parameter default to false (tolerateMissingFieldValues =
false) so callers must opt-in, keep the existing conditional that adds
MakeFieldsNullableTypeInterceptor only when tolerateMissingFieldValues is true,
and update any related tests/docs/examples that assumed the previous default to
explicitly pass true where needed; reference the UseGraphQL<TQuery>() signature,
the tolerateMissingFieldValues parameter, and the
MakeFieldsNullableTypeInterceptor addition in your changes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3ab3a0e2-9c52-45b3-b4f0-daf3c334ce43
📒 Files selected for processing (2)
src/Simplic.OxS.Server/Extensions/GraphQLExtension.cssrc/Simplic.OxS.Server/GraphQL/MakeFieldsNullableTypeInterceptor.cs
| Action<IRequestExecutorBuilder> builder = null, | ||
| bool tolerateMissingFieldValues = true) where TQuery : class |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify nullable context + detect similar non-nullable delegate defaults to null.
fd -e csproj --exec rg -n "<Nullable>.*</Nullable>" {}
rg -nP --type=cs '\bAction<[^>]+>\s+\w+\s*=\s*null\b'Repository: simplic/simplic-oxs
Length of output: 1195
🏁 Script executed:
cat -n src/Simplic.OxS.Server/Extensions/GraphQLExtension.cs | head -50Repository: simplic/simplic-oxs
Length of output: 2607
Mark the optional builder callback as nullable.
Line 24 assigns null to a non-nullable delegate type. Use Action<IRequestExecutorBuilder>? to align with the enabled nullable reference type context.
Suggested fix
- Action<IRequestExecutorBuilder> builder = null,
+ Action<IRequestExecutorBuilder>? builder = null,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Action<IRequestExecutorBuilder> builder = null, | |
| bool tolerateMissingFieldValues = true) where TQuery : class | |
| Action<IRequestExecutorBuilder>? builder = null, | |
| bool tolerateMissingFieldValues = true) where TQuery : class |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/Simplic.OxS.Server/Extensions/GraphQLExtension.cs` around lines 24 - 25,
The method parameter 'builder' in GraphQLExtension.cs is being assigned null but
is declared non-nullable; change its type to a nullable delegate by updating the
parameter declaration from Action<IRequestExecutorBuilder> builder = null to
Action<IRequestExecutorBuilder>? builder = null (i.e., make the 'builder'
parameter nullable) so it aligns with nullable reference types and the method
signature (where TQuery : class).
| if (runtimeType?.Namespace is { } ns && | ||
| ns.StartsWith("HotChocolate", System.StringComparison.Ordinal)) | ||
| { |
There was a problem hiding this comment.
Tighten the namespace guard to avoid skipping user types.
Line 36 uses StartsWith("HotChocolate", ...), which also matches namespaces like HotChocolateExtensions and can unintentionally bypass this interceptor for user-defined runtime types.
Suggested fix
- if (runtimeType?.Namespace is { } ns &&
- ns.StartsWith("HotChocolate", System.StringComparison.Ordinal))
+ if (runtimeType?.Namespace is { } ns &&
+ (ns.Equals("HotChocolate", System.StringComparison.Ordinal) ||
+ ns.StartsWith("HotChocolate.", System.StringComparison.Ordinal)))
{
return;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (runtimeType?.Namespace is { } ns && | |
| ns.StartsWith("HotChocolate", System.StringComparison.Ordinal)) | |
| { | |
| if (runtimeType?.Namespace is { } ns && | |
| (ns.Equals("HotChocolate", System.StringComparison.Ordinal) || | |
| ns.StartsWith("HotChocolate.", System.StringComparison.Ordinal))) | |
| { |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/Simplic.OxS.Server/GraphQL/MakeFieldsNullableTypeInterceptor.cs` around
lines 35 - 37, The namespace check in MakeFieldsNullableTypeInterceptor is too
broad: the runtimeType?.Namespace StartsWith("HotChocolate", ...) will also
match names like "HotChocolateExtensions" and skip user types; change the guard
to only skip the exact HotChocolate root or its subnamespaces by checking either
Namespace.Equals("HotChocolate", StringComparison.Ordinal) ||
Namespace.StartsWith("HotChocolate.", StringComparison.Ordinal) (or the
equivalent) so runtimeType.Namespace only bypasses the interceptor for
"HotChocolate" and "HotChocolate.*".
1 participant
Summary by CodeRabbit