Skip to content

[dotnet] Don't truncate internal log messages at error/warn levels#17333

Merged
nvborisenko merged 3 commits intoSeleniumHQ:trunkfrom
nvborisenko:dotnet-log-important-truncate
Apr 10, 2026
Merged

[dotnet] Don't truncate internal log messages at error/warn levels#17333
nvborisenko merged 3 commits intoSeleniumHQ:trunkfrom
nvborisenko:dotnet-log-important-truncate

Conversation

@nvborisenko
Copy link
Copy Markdown
Member

To see entire error/warn log message for tracebility.

💥 What does this PR do?

This pull request makes a targeted improvement to the logging behavior in LogContext.cs. Now, log messages are only truncated if a truncation length is set and the log level is below Warn. This ensures that important warning and error messages are not truncated, preserving their full content for debugging.

Logging behavior update:

  • Modified the EmitMessage method in LogContext.cs so that message truncation only occurs for log levels below Warn and when a truncation length is specified. ([dotnet/src/webdriver/Internal/Logging/LogContext.csL101-R106](https://github.com/SeleniumHQ/selenium/pull/17333/files#diff-09e55ca02fe7a68ac190114b4bcbe9bc5f68d0b641f226bd17ffaf2a81d465b9L101-R106))

🔄 Types of changes

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

Copilot AI review requested due to automatic review settings April 10, 2026 09:46
@qodo-code-review
Copy link
Copy Markdown
Contributor

Review Summary by Qodo

Prevent truncation of warning and error level log messages

✨ Enhancement

Grey Divider

Walkthroughs

Description
• Prevent truncation of warning and error log messages
• Only truncate logs below warn level when configured
• Preserve full content for important error diagnostics

Grey Divider

File Changes

1. dotnet/src/webdriver/Internal/Logging/LogContext.cs ✨ Enhancement +5/-2

Conditional message truncation based on log level

• Modified EmitMessage method to conditionally truncate messages
• Messages at Warn level and above are no longer truncated
• Truncation now only applies to log levels below Warn when _truncationLength is set
• Improved traceability for important error and warning messages

dotnet/src/webdriver/Internal/Logging/LogContext.cs


Grey Divider

Qodo Logo

@qodo-code-review
Copy link
Copy Markdown
Contributor

qodo-code-review bot commented Apr 10, 2026

Code Review by Qodo

🐞 Bugs (1)   📘 Rule violations (0)   📎 Requirement gaps (0)   🎨 UX Issues (0)
🐞\ ➹ Performance (1)

Grey Divider


Action required

1. EmitMessage Warn/Error truncation untested📘
Description
The PR changes observable logging behavior by skipping truncation for Warn and higher levels, but
no corresponding unit tests were added/updated to lock this behavior in. This risks regression where
important Warn/Error messages could be truncated again without detection.
Code

dotnet/src/webdriver/Internal/Logging/LogContext.cs[R101-106]

+            if (_truncationLength.HasValue && level < LogEventLevel.Warn)
+            {
+                message = TruncateMessage(message, _truncationLength);
+            }

-            var logEvent = new LogEvent(logger.Issuer, timestamp, level, truncatedMessage);
+            var logEvent = new LogEvent(logger.Issuer, timestamp, level, message);
Evidence
Compliance requires tests for behavior changes. EmitMessage now truncates only when
_truncationLength is set and level < LogEventLevel.Warn, but existing tests only assert
truncation behavior for Info and for truncation being disabled, with no assertions covering the
new Warn/Error non-truncation behavior.

AGENTS.md
dotnet/src/webdriver/Internal/Logging/LogContext.cs[101-106]
dotnet/test/webdriver/Internal/Logging/LogTests.cs[248-300]
Best Practice: Learned patterns

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`EmitMessage` behavior changed to not truncate messages at `Warn`/`Error`, but unit tests were not updated to assert this new behavior.

## Issue Context
There are existing truncation tests in `LogTests.cs`, but they only validate `Info`-level truncation and the case when truncation is disabled; they do not cover the new level-based truncation rule.

## Fix Focus Areas
- dotnet/src/webdriver/Internal/Logging/LogContext.cs[101-106]
- dotnet/test/webdriver/Internal/Logging/LogTests.cs[248-300]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

2. Unbounded warn/error output 🐞
Description
EmitMessage now skips truncation for Warn/Error levels even when a truncation length is configured,
which makes the default 1000-char truncation cap ineffective for the default Warn-level global
context. Call sites log entire remote payloads at Warn (e.g., BiDi Broker includes full message
content), so a large payload can cause very large stderr/file writes and significant I/O/lock
contention.
Code

dotnet/src/webdriver/Internal/Logging/LogContext.cs[R101-106]

+            if (_truncationLength.HasValue && level < LogEventLevel.Warn)
+            {
+                message = TruncateMessage(message, _truncationLength);
+            }

-            var logEvent = new LogEvent(logger.Issuer, timestamp, level, truncatedMessage);
+            var logEvent = new LogEvent(logger.Issuer, timestamp, level, message);
Evidence
The new conditional only truncates for levels below Warn, while the global context defaults to Warn
and sets truncation to 1000. As a result, default Warn logs are no longer truncated. Some Warn log
call sites embed full remote message contents (potentially large), which will now be emitted in
full.

dotnet/src/webdriver/Internal/Logging/LogContext.cs[97-112]
dotnet/src/webdriver/Internal/Logging/LogContextManager.cs[24-45]
dotnet/src/webdriver/BiDi/Broker.cs[288-367]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`LogContext.EmitMessage` no longer truncates Warn/Error messages even when `_truncationLength` is set. With the default global context configured as `Warn` and `DefaultTruncationLength = 1000`, the default truncation protection is effectively disabled for the most common emitted levels (Warn/Error), allowing very large payloads to be written to stderr/files.

### Issue Context
Some Warn log sites embed full remote message content (e.g., BiDi Broker logs `Encoding.UTF8.GetString(...)`), so skipping truncation can lead to extremely large log lines and heavy I/O/locking.

### Fix Focus Areas
- dotnet/src/webdriver/Internal/Logging/LogContext.cs[97-112]
- dotnet/src/webdriver/Internal/Logging/LogContextManager.cs[24-45]
- dotnet/src/webdriver/BiDi/Broker.cs[288-367]

### What to change
Implement one of:
1) Introduce a separate, higher hard cap for Warn/Error (e.g., `MaxWarnErrorLength`) still preventing pathological sizes.
2) Make truncation threshold configurable (e.g., truncate up to `Warn` by default, allow opting out).
3) Keep the new behavior but add an explicit opt-in flag to disable truncation for Warn/Error, rather than making it unconditional for everyone with truncation enabled by default.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

ⓘ The new review experience is currently in Beta. Learn more

Grey Divider

Qodo Logo

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adjusts .NET internal logging so truncation does not apply to warning/error messages, preserving full content for higher-severity logs.

Changes:

  • Update LogContext.EmitMessage to only truncate when a truncation length is set and level < Warn.
  • Ensure Warn/Error messages are emitted without truncation even when truncation is configured.

@nvborisenko nvborisenko merged commit 9fb6b18 into SeleniumHQ:trunk Apr 10, 2026
19 checks passed
@nvborisenko nvborisenko deleted the dotnet-log-important-truncate branch April 10, 2026 17:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C-dotnet .NET Bindings

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants