[dotnet] [bidi] Make ContinueWithAuthCommand closer to spec (breaking change)

[nvborisenko]

User description

Motivation and Context

Revisited ContinueWithAuthCommand to be closer with spec, fixes #15539.
Also avoided nested types, opening the doors for static factory.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist

• I have read the contributing document.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have added tests to cover my changes.
• All new and existing tests passed.

---

PR Type

Enhancement, Tests

---

Description

• Refactored ContinueWithAuth method to align with BiDi spec.

• Introduced new classes for clearer type differentiation.

• Updated method overloads for better usability and clarity.

• Adjusted test cases to validate the refactored implementation.

---

Changes walkthrough 📝

	Relevant files
Enhancement	BiDiJsonSerializerContext.cs Simplified serialization context for network commands        dotnet/src/webdriver/BiDi/Communication/Json/BiDiJsonSerializerContext.cs Removed unused JsonSerializable type for Default action. Simplified serialization context for network commands. +0/-1      ContinueWithAuthCommand.cs Refactored `ContinueWithAuthCommand` with clearer abstractions dotnet/src/webdriver/BiDi/Modules/Network/ContinueWithAuthCommand.cs Replaced nested types with standalone records for clarity. Introduced ContinueWithAuthNoCredentials abstraction. Added specific records for default and cancel actions. Updated options classes to match new record structure. +16/-11  NetworkModule.cs Updated `ContinueWithAuthAsync` method overloads                  dotnet/src/webdriver/BiDi/Modules/Network/NetworkModule.cs Updated ContinueWithAuthAsync method overloads. Adjusted parameter types to match new record structure. Improved method naming for better alignment with spec. +6/-6      Request.cs Adjusted `Request` methods for refactored auth handling    dotnet/src/webdriver/BiDi/Modules/Network/Request.cs Updated ContinueWithAuthAsync calls to use new options classes. Improved method signatures for consistency with refactored commands. +3/-3
Tests	NetworkTest.cs Updated tests for refactored `ContinueWithAuth` implementation dotnet/test/common/BiDi/Network/NetworkTest.cs Updated test cases to use new options classes. Validated ContinueWithAuth behavior for all scenarios. Removed outdated comments for better clarity. +2/-3

---

Need help?

Type /help how to ... in the comments thread for any questions about Qodo Merge usage.

Check out the documentation for more information.

[qodo-merge-pro]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis ✅ 15539 - Fully compliant Compliant requirements: • Rework ContinueWithAuth method to align with BiDi spec • Apply method overloading for different authentication actions • Support the three actions defined in spec: "provideCredentials", "default", and "cancel" • Improve code structure to be closer to the spec definition

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Empty Test Block The test method for default credentials has an empty code block in the event handler. This might be a mistake as the ContinueWithAuthAsync call is present but commented out in the old code and added back in the new code. await using var intercept = await bidi.Network.InterceptAuthAsync(async e => { await e.Request.Request.ContinueWithAuthAsync(new ContinueWithAuthDefaultCredentialsOptions()); });

[qodo-merge-pro]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Add missing JSON property annotation The property name for Credentials doesn't match the expected JSON property name. According to the removed code, the property should be serialized as "credentials" but the new implementation doesn't specify this, which could cause serialization issues. dotnet/src/webdriver/BiDi/Modules/Network/ContinueWithAuthCommand.cs [34] -internal record ContinueWithAuthCredentials(Request Request, AuthCredentials Credentials) : ContinueWithAuthParameters(Request); +internal record ContinueWithAuthCredentials(Request Request, [property: JsonPropertyName("credentials")] AuthCredentials Credentials) : ContinueWithAuthParameters(Request); Apply this suggestion Suggestion importance[1-10]: 9 __ Why: The suggestion correctly identifies a critical issue where the JSON property name annotation for the Credentials property was lost during refactoring. Without this annotation, the serialized JSON would use "Credentials" instead of "credentials", breaking API compatibility.	High
	Fix options type casting The method is passing options directly to ExecuteCommandAsync, but the options parameter type has changed from ContinueWithAuthOptions to the more specific ContinueWithAuthCredentialsOptions. This could cause issues if the base command execution logic expects the base options type. dotnet/src/webdriver/BiDi/Modules/Network/NetworkModule.cs [107-110] internal async Task ContinueWithAuthAsync(Request request, AuthCredentials credentials, ContinueWithAuthCredentialsOptions? options = null) { - await Broker.ExecuteCommandAsync(new ContinueWithAuthCommand(new ContinueWithAuthCredentials(request, credentials)), options).ConfigureAwait(false); + await Broker.ExecuteCommandAsync(new ContinueWithAuthCommand(new ContinueWithAuthCredentials(request, credentials)), options as ContinueWithAuthOptions).ConfigureAwait(false); } Apply this suggestion Suggestion importance[1-10]: 7 __ Why: This suggestion addresses a potential runtime issue where the more specific options type might not be compatible with what the ExecuteCommandAsync method expects. Casting to the base type ensures type compatibility while maintaining the improved type safety of the refactored code.	Medium
Learned best practice	Add null parameter validation to prevent potential NullReferenceExceptions The method accepts potentially null parameters (request and credentials) but doesn't validate them before use. Add null checks using ArgumentNullException.ThrowIfNull() to prevent potential NullReferenceExceptions when these parameters are accessed. dotnet/src/webdriver/BiDi/Modules/Network/NetworkModule.cs [107-110] internal async Task ContinueWithAuthAsync(Request request, AuthCredentials credentials, ContinueWithAuthCredentialsOptions? options = null) { + ArgumentNullException.ThrowIfNull(request, nameof(request)); + ArgumentNullException.ThrowIfNull(credentials, nameof(credentials)); await Broker.ExecuteCommandAsync(new ContinueWithAuthCommand(new ContinueWithAuthCredentials(request, credentials)), options).ConfigureAwait(false); } Apply this suggestion Suggestion importance[1-10]: 6	Low
Update

[RenderMichael]

I like the pattern!