Skip to content

[dotnet] [bidi] AOT safe json converter for OptionalConverter#16963

Merged
nvborisenko merged 2 commits intoSeleniumHQ:trunkfrom
nvborisenko:bidi-aot-optional-converter
Jan 20, 2026
Merged

[dotnet] [bidi] AOT safe json converter for OptionalConverter#16963
nvborisenko merged 2 commits intoSeleniumHQ:trunkfrom
nvborisenko:bidi-aot-optional-converter

Conversation

@nvborisenko
Copy link
Member

@nvborisenko nvborisenko commented Jan 20, 2026

User description

💥 What does this PR do?

Resolves AOT trimming warning for OptionalConverter.

🔄 Types of changes

  • Cleanup (formatting, renaming)

PR Type

Bug fix


Description

  • Replace generic JsonSerializer calls with AOT-safe type info overloads

  • Use options.GetTypeInfo<T>() for deserialization and serialization

  • Resolves AOT trimming warnings in OptionalConverter


Diagram Walkthrough

flowchart LR
  A["OptionalConverter<br/>Deserialization"] -->|"Replace generic call"| B["GetTypeInfo<T><br/>approach"]
  C["OptionalConverter<br/>Serialization"] -->|"Replace generic call"| B
  B -->|"Result"| D["AOT trimming<br/>warnings resolved"]
Loading

File Walkthrough

Relevant files
Bug fix
OptionalConverter.cs
Replace generic JSON serializer calls with AOT-safe overloads

dotnet/src/webdriver/BiDi/Json/Converters/OptionalConverter.cs

  • Updated Read() method to use options.GetTypeInfo() instead of generic
    JsonSerializer.Deserialize()
  • Updated Write() method to use options.GetTypeInfo() instead of generic
    JsonSerializer.Serialize()
  • Eliminates AOT trimming warnings by providing explicit type
    information to the serializer
+2/-2     

@qodo-code-review
Copy link
Contributor

qodo-code-review bot commented Jan 20, 2026

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢
No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
🟢
Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

  • Update
Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

@qodo-code-review
Copy link
Contributor

qodo-code-review bot commented Jan 20, 2026

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Add null checks for type info

In both the Read and Write methods, add a null check for the result of
options.GetTypeInfo() and throw a NotSupportedException if it is null to provide
a clearer error message.

dotnet/src/webdriver/BiDi/Json/Converters/OptionalConverter.cs [28-50]

 public override Optional<T> Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
 {
     if (reader.TokenType == JsonTokenType.Null)
     {
         reader.Read(); // consume null
         return new Optional<T>(default!);
     }
 
-    T value = JsonSerializer.Deserialize(ref reader, options.GetTypeInfo<T>())!;
+    var typeInfo = options.GetTypeInfo<T>();
+    if (typeInfo is null)
+    {
+        throw new NotSupportedException($"The type '{typeof(T)}' is not configured for JSON serialization. Ensure it is included in a source-generated context.");
+    }
+
+    T value = JsonSerializer.Deserialize(ref reader, typeInfo)!;
     return new Optional<T>(value);
 }
 
 public override void Write(Utf8JsonWriter writer, Optional<T> value, JsonSerializerOptions options)
 {
     if (value.TryGetValue(out var optionalValue))
     {
-        JsonSerializer.Serialize(writer, optionalValue, options.GetTypeInfo<T>());
+        var typeInfo = options.GetTypeInfo<T>();
+        if (typeInfo is null)
+        {
+            throw new NotSupportedException($"The type '{typeof(T)}' is not configured for JSON serialization. Ensure it is included in a source-generated context.");
+        }
+
+        JsonSerializer.Serialize(writer, optionalValue, typeInfo);
     }
     else
     {
         writer.WriteNullValue();
     }
 }

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies that options.GetTypeInfo<T>() can return null and proposes adding a null check with a more descriptive exception, which improves the robustness and debuggability of the code.

Medium
  • Update

@nvborisenko nvborisenko merged commit 06d9a14 into SeleniumHQ:trunk Jan 20, 2026
14 checks passed
@nvborisenko nvborisenko deleted the bidi-aot-optional-converter branch January 20, 2026 18:30
This was referenced Feb 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants