[dotnet][bidi] Remove json serialization from transport layer

[nvborisenko]

User description

Motivation and Context

Simplify transport layer to know nothing about json serialization context
.

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

• Removed JSON serialization from the transport layer.

• Introduced raw byte data handling for sending/receiving.

• Simplified ITransport interface by removing JSON-specific methods.

• Updated Broker and WebSocketTransport to use new transport methods.

---

Changes walkthrough 📝

Relevant files

Enhancement

Broker.cs Refactored Broker to remove JSON serialization dependency dotnet/src/webdriver/BiDi/Communication/Broker.cs Replaced JSON-specific transport methods with raw byte handling. Updated message deserialization to use JsonSerializer.Deserialize. Adjusted command execution to serialize/deserialize manually. Fixed formatting in generic constraints. +11/-7    ITransport.cs Simplified ITransport interface for raw byte handling        dotnet/src/webdriver/BiDi/Communication/Transport/ITransport.cs Removed JSON-specific methods from ITransport interface. Added methods for raw byte data handling. +2/-4      WebSocketTransport.cs Updated WebSocketTransport to handle raw byte data              dotnet/src/webdriver/BiDi/Communication/Transport/WebSocketTransport.cs Replaced JSON serialization with raw byte handling. Updated logging to reflect raw byte data. Adjusted WebSocket send/receive methods for new interface. +8/-13

---

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-code-review]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

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

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Error Handling The deserialization of messages using JsonSerializer.Deserialize lacks error handling for malformed JSON data. Consider adding try-catch block to handle potential JsonException. var message = JsonSerializer.Deserialize(new ReadOnlySpan<byte>(data), _jsonSerializerContext.Message); Resource Leak The MemoryStream is disposed but the received data is stored in a byte array that persists. Consider using ArrayPool for better memory management of large messages. byte[] data = ms.ToArray();

[qodo-code-review]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Restore long-running task option Add TaskCreationOptions.LongRunning back to the task factory calls since these are long-running background operations that shouldn't use the thread pool. dotnet/src/webdriver/BiDi/Communication/Broker.cs [114-115] -_receivingMessageTask = _myTaskFactory.StartNew(async () => await ReceiveMessagesAsync(_receiveMessagesCancellationTokenSource.Token)).Unwrap(); -_eventEmitterTask = _myTaskFactory.StartNew(ProcessEventsAwaiterAsync).Unwrap(); +_receivingMessageTask = _myTaskFactory.StartNew(async () => await ReceiveMessagesAsync(_receiveMessagesCancellationTokenSource.Token), TaskCreationOptions.LongRunning).Unwrap(); +_eventEmitterTask = _myTaskFactory.StartNew(ProcessEventsAwaiterAsync, TaskCreationOptions.LongRunning).Unwrap(); Apply this suggestion Suggestion importance[1-10]: 8 __ Why: The PR removed TaskCreationOptions.LongRunning which is important for these background tasks as they are continuous operations. Without this option, the tasks will use thread pool threads which could impact performance and scalability.	Medium
	Add null check after deserialization Add null check for the deserialized message before the switch statement to prevent potential NullReferenceException. dotnet/src/webdriver/BiDi/Communication/Broker.cs [124-126] var message = JsonSerializer.Deserialize(new ReadOnlySpan<byte>(data), _jsonSerializerContext.Message); +if (message == null) +{ + throw new InvalidOperationException("Failed to deserialize message"); +} switch (message) Apply this suggestion Suggestion importance[1-10]: 7 __ Why: Adding a null check with proper error handling is important to prevent potential NullReferenceException and provide a clear error message when deserialization fails.	Medium
Learned best practice	Add null validation checks for required method parameters to prevent NullReferenceExceptions The ExecuteCommandAsync and ExecuteCommandCoreAsync methods accept a nullable command parameter but don't validate it for null. Add null validation at the start of these methods to prevent potential NullReferenceException when accessing command properties. dotnet/src/webdriver/BiDi/Communication/Broker.cs [183-186] public async Task<TResult> ExecuteCommandAsync<TCommand, TResult>(TCommand command, CommandOptions? options) where TCommand : Command { + ArgumentNullException.ThrowIfNull(command, nameof(command)); var jsonElement = await ExecuteCommandCoreAsync(command, options).ConfigureAwait(false); Apply this suggestion	Low

[RenderMichael]

I like this simplification!