Skip to content

[dotnet] Correlate low level http requests/responses and log failures - #17074

Merged
nvborisenko merged 3 commits into
SeleniumHQ:trunkfrom
nvborisenko:dotnet-http-req-res-correlation
Feb 10, 2026
Merged

nvborisenko merged 3 commits into
SeleniumHQ:trunkfrom
nvborisenko:dotnet-http-req-res-correlation

Conversation

@nvborisenko

@nvborisenko nvborisenko commented Feb 10, 2026 •

Copy link
Copy Markdown
Member
  • Each HTTP request is now assigned a unique requestId (derived from the request's hash code), which is included in both request and response log messages for easier correlation.

Error handling enhancements:

  • If an exception occurs during the HTTP request, a trace log including the requestId and exception details is recorded before rethrowing the exception.

🔗 Related Issues

Contribute to #17053

🔄 Types of changes

  • New feature (non-breaking change which adds functionality and tests!)

Copilot AI review requested due to automatic review settings February 10, 2026 19:58
@qodo-code-review

Copy link
Copy Markdown
Contributor

PR Type

Enhancement


Description

  • Add unique requestId to HTTP request/response log messages for correlation

  • Include requestId, HTTP method, URI, status code in trace logs

  • Log exceptions with requestId before rethrowing during HTTP failures

  • Improve HTTP request/response traceability and debugging capabilities


File Walkthrough

Relevant files
Enhancement
HttpCommandExecutor.cs
Add request ID correlation to HTTP logging                             

dotnet/src/webdriver/Remote/HttpCommandExecutor.cs

  • Generate unique requestId from request hash code for each HTTP request
  • Include requestId in request log messages alongside method and URI
  • Include requestId in response log messages alongside status code and
    reason phrase
  • Add exception handling to log failures with requestId before
    rethrowing
+23/-4   

@qodo-code-review

qodo-code-review Bot commented Feb 10, 2026 •

Copy link
Copy Markdown
Contributor

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
⚪
Sensitive log exposure

Description: Trace logging includes full HTTP request/response details (and exception text) with
request.Content.ReadAsStringAsync() and ex.ToString() (lines 453-455 and 471), which may
leak sensitive data (e.g., session IDs, auth tokens, cookies, URLs with credentials) into
logs when Trace is enabled. HttpCommandExecutor.cs [438-485]

Referred Code
            string requestId = request.GetHashCode().ToString("x8");

            if (_logger.IsEnabled(LogEventLevel.Trace))
            {
                StringBuilder requestLogMessageBuilder = new();
                requestLogMessageBuilder.AppendFormat(">> [{0}] {1} {2}",
                    requestId,
                    request.Method,
                    request.RequestUri?.ToString() ?? "null");

                if (request.Content is not null)
                {
#if NET8_0_OR_GREATER
                    var requestContent = await request.Content.ReadAsStringAsync(cancellationToken).ConfigureAwait(false);
#else
                    var requestContent = await request.Content.ReadAsStringAsync().ConfigureAwait(false);
#endif
                    requestLogMessageBuilder.AppendFormat("{0}{1}", Environment.NewLine, requestContent);
                }

                _logger.Trace(requestLogMessageBuilder.ToString());


 ... (clipped 27 lines)
Ticket Compliance
🟡
🎫 #17053
🟢 Provide enough diagnostics/logging to debug the reported "driver connection may be lost"
failures (HTTP request failures to the local WebDriver server).
🔴 Prevent or properly handle loss of WebDriver connection in .NET when a website closes a
pop-up window and subsequent calls like driver.WindowHandles are made.
Ensure switching/navigating back to the original window remains reliable after a
non-driver-initiated pop-up close.
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: 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

🔴
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:
Logs full exception: The new trace statement _logger.Trace($"<< [{requestId}] {ex}") logs the
full exception (including message/stack trace), which can leak secrets or sensitive data
into logs.

Referred Code
if (_logger.IsEnabled(LogEventLevel.Trace))
{
    _logger.Trace($"<< [{requestId}] {ex}");
}

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:
Exception detail logged: The new trace log writes the full exception (ex) which may include sensitive internal
details depending on upstream exception messages and should be confirmed as internal-only
and appropriately redacted.

Referred Code
catch (Exception ex)
{
    if (_logger.IsEnabled(LogEventLevel.Trace))
    {
        _logger.Trace($"<< [{requestId}] {ex}");
    }

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

qodo-code-review Bot commented Feb 10, 2026 •

Copy link
Copy Markdown
Contributor

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Buffer request content before logging

Add a call to request.Content.LoadIntoBufferAsync() before reading the request
content for logging to prevent consuming the stream and sending an empty
payload.

dotnet/src/webdriver/Remote/HttpCommandExecutor.cs [448-456]

       if (request.Content is not null)
       {
+          await request.Content.LoadIntoBufferAsync().ConfigureAwait(false);
   #if NET8_0_OR_GREATER
           var requestContent = await request.Content.ReadAsStringAsync(cancellationToken).ConfigureAwait(false);
   #else
           var requestContent = await request.Content.ReadAsStringAsync().ConfigureAwait(false);
   #endif
           requestLogMessageBuilder.AppendFormat("{0}{1}", Environment.NewLine, requestContent);
       }
  • Apply / Chat
Suggestion importance[1-10]: 10

__

Why: This suggestion identifies a critical bug where logging the request body consumes the content stream, which would likely result in an empty request body being sent. The proposed fix using LoadIntoBufferAsync is correct.

High
Use a GUID for unique IDs

Replace request.GetHashCode() with a shortened Guid.NewGuid() to generate a more
reliable unique request ID for log correlation.

dotnet/src/webdriver/Remote/HttpCommandExecutor.cs [438]

-string requestId = request.GetHashCode().ToString("x8");
+string requestId = Guid.NewGuid().ToString("N")[..8];
  • Apply / Chat
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly points out that GetHashCode() does not guarantee uniqueness, which undermines the request correlation feature being added. Using a GUID is a much more robust approach for generating unique request IDs.

Medium
General
✅ Use a distinct log prefix for failures
Suggestion Impact:The exception trace log prefix was changed from "<<" to "!!" in the catch block, distinguishing failure logs from successful response logs (though without adding the suggested "Request failed:" text).

code diff:

@@ -466,19 +467,19 @@
             }
             catch (Exception ex)
             {
-                if (_logger.IsEnabled(LogEventLevel.Trace))
+                if (isTracingEnabled)
                 {
-                    _logger.Trace($"<< [{requestId}] {ex}");
+                    _logger.Trace($"!! [{correlationId}] {ex}");
                 }
 
                 throw;
             }

Use a different log prefix for exceptions, such as !!, to distinguish them from
successful response logs which use <<.

dotnet/src/webdriver/Remote/HttpCommandExecutor.cs [467-475]

 catch (Exception ex)
 {
     if (_logger.IsEnabled(LogEventLevel.Trace))
     {
-        _logger.Trace($"<< [{requestId}] {ex}");
+        _logger.Trace($"!! [{requestId}] Request failed: {ex}");
     }
 
     throw;
 }

[Suggestion processed]

Suggestion importance[1-10]: 4

__

Why: This is a good suggestion for improving log clarity by using a distinct prefix for exceptions versus successful responses, which makes debugging easier.

Low
  • Update

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR improves trace-level diagnostics in the .NET HttpCommandExecutor transport by adding a correlation identifier to link each low-level HTTP request log to its corresponding response (and to trace exceptions during send).

Changes:

  • Add a requestId and include it in trace logs for outgoing HTTP requests and incoming HTTP responses.
  • Wrap the HTTP send in a try/catch to trace exceptions (including requestId) before rethrowing.

Comment thread dotnet/src/webdriver/Remote/HttpCommandExecutor.cs Outdated
Comment thread dotnet/src/webdriver/Remote/HttpCommandExecutor.cs Outdated
Comment thread dotnet/src/webdriver/Remote/HttpCommandExecutor.cs Outdated
Copilot AI review requested due to automatic review settings February 10, 2026 20:14

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

Comment thread dotnet/src/webdriver/Remote/HttpCommandExecutor.cs
@nvborisenko
nvborisenko merged commit 4ba0f31 into SeleniumHQ:trunk Feb 10, 2026
19 checks passed
@nvborisenko
nvborisenko deleted the dotnet-http-req-res-correlation branch February 10, 2026 20:44
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.

3 participants