[dotnet] Move Response constructors towards immutability

[RenderMichael]

User description

Thanks for contributing to Selenium!
A PR well described will help maintainers to quickly review and merge it

Before submitting your PR, please check our contributing guidelines.
Avoid large PRs, help reviewers by making them as simple and short as possible.

Description

This moves all Response constructors to a single constructor which requires all values specified at ctor time.

Motivation and Context

Moves us towards immutability of the Response type. Eventually, we may be able to remove the setters from some values, but that's not crucial.

This also moves us towards aspirational goals of storing a JsonNode for the Value alongside the de-serialized object, which would help with bugs such as #14846 and many others involving incorrect de-serialization, especially with .ToString() invocations on dictionaries/lists, and as casts when the type is not a match.

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

---

Description

• New immutable Response constructor requiring all values

• Refactored response creation methods for better immutability

• Improved error handling and null checks

• Simplified JSON deserialization and response construction

---

Changes walkthrough 📝

Relevant files

Enhancement

HttpCommandExecutor.cs Refactor response creation to use immutable constructor    dotnet/src/webdriver/Remote/HttpCommandExecutor.cs Refactored CreateResponse method to use new Response constructor Simplified error handling and response creation logic Improved string value handling with pattern matching +5/-6      Response.cs Implement immutable Response construction pattern                dotnet/src/webdriver/Response.cs Added new constructor requiring all values upfront Refactored FromJson and FromErrorJson methods to use new constructor Improved null handling and error checking Simplified response object creation logic +37/-35

---

💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

[qodo-merge-pro]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶 14846 - Partially compliant Compliant requirements: The PR moves towards fixing incorrect deserialization and ToString() invocations on dictionaries by refactoring Response class to support storing JsonNode alongside deserialized objects Non-compliant requirements: The specific bug with GetDomProperty("style") is not directly fixed in this PR Requires further human verification: Need to verify if the Response class changes actually help resolve the style property deserialization issue

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

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Null Handling Verify that null handling in the new constructor and FromJson method is comprehensive and handles all edge cases correctly public Response(string sessionId, object value, WebDriverResult status) { this.SessionId = sessionId; this.Value = value; this.Status = status; } Type Conversion Ensure dictionary value conversions and type casting in FromJson method properly handle all possible response formats if (contents is Dictionary<string, object> valueDictionary) { // Special case code for the new session command. If the response contains // sessionId and capabilities properties, fix up the session ID and value members. if (valueDictionary.TryGetValue("sessionId", out var session)) { sessionId = session.ToString(); if (valueDictionary.TryGetValue("capabilities", out object capabilities)) { contents = capabilities; } else { contents = valueDictionary["value"]; } } }

[qodo-merge-pro]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Score
Possible issue	✅ Add parameter validation to prevent null reference exceptions Suggestion Impact: The commit adds XML documentation to indicate that FromJson can throw ArgumentNullException for null values, though it relies on JsonSerializer's built-in null validation rather than adding an explicit check code diff: + /// <exception cref="ArgumentNullException">If <paramref name="value"/> is <see langword="null"/>.</exception> + /// <exception cref="JsonException">If <paramref name="value"/> is not a valid JSON object.</exception> Add null check for value parameter in FromJson method to prevent potential NullReferenceException when deserializing JSON. dotnet/src/webdriver/Response.cs [74-76] public static Response FromJson(string value) { + if (string.IsNullOrEmpty(value)) + { + throw new ArgumentNullException(nameof(value)); + } Dictionary<string, object> rawResponse = JsonSerializer.Deserialize<Dictionary<string, object>>(value, s_jsonSerializerOptions) Apply this suggestion Suggestion importance[1-10]: 7 Why: Adding null validation for the input parameter is important for robustness and would prevent potential runtime errors. The current code assumes valid input which could lead to unclear error messages if null is passed.	7
	Initialize variables at declaration to prevent potential null reference exceptions Initialize the response variable when declaring it to avoid potential null reference issues. Since it's assigned in all code paths, it should be initialized with a default value or declared just before its first assignment. dotnet/src/webdriver/Remote/HttpCommandExecutor.cs [319-321] -Response response; +Response response = new Response(sessionId: null, string.Empty, WebDriverResult.Success); string body = responseInfo.Body; if ((int)responseInfo.StatusCode < 200 || (int)responseInfo.StatusCode > 299) Apply this suggestion Suggestion importance[1-10]: 2 Why: While initializing variables at declaration is generally good practice, in this case the variable is assigned in all code paths and the PR's refactoring already handles this safely. The suggestion would add unnecessary object creation.	2