Skip to content

[dotnet] Enabling drivers to set log to console. #16097

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 17 commits into
base: trunk
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 0 additions & 22 deletions dotnet/src/webdriver/DriverProcessStartedEventArgs.cs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@

using System;
using System.Diagnostics;
using System.IO;

namespace OpenQA.Selenium;

Expand All @@ -41,31 +40,10 @@ public DriverProcessStartedEventArgs(Process driverProcess)
}

this.ProcessId = driverProcess.Id;
if (driverProcess.StartInfo.RedirectStandardOutput && !driverProcess.StartInfo.UseShellExecute)
Copy link
Member Author

Choose a reason for hiding this comment

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

This might be to aggressive but since we don't want to do sync reads, we might delete it. Or deprecate first?

Copy link
Member Author

Choose a reason for hiding this comment

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

I did a quick search in GitHub and other places for DriverProcessStartedEventArgs and did not see mentions.

Copy link
Member

Choose a reason for hiding this comment

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

I was surprised here too. My initial thoughts were "no, don't expose it, it is so internal".

I propose to... not sure what. Let's leave this thread unresolved.

May be:

  • deep dive and find correct combination of conditions when we are safe
  • aj, remove it..

Let's wait little bit.

{
this.StandardOutputStreamReader = driverProcess.StandardOutput;
}

if (driverProcess.StartInfo.RedirectStandardError && !driverProcess.StartInfo.UseShellExecute)
{
this.StandardErrorStreamReader = driverProcess.StandardError;
}
}

/// <summary>
/// Gets the unique ID of the driver executable process.
/// </summary>
public int ProcessId { get; }

/// <summary>
/// Gets a <see cref="StreamReader"/> object that can be used to read the contents
/// printed to <c>stdout</c> by a driver service process.
/// </summary>
public StreamReader? StandardOutputStreamReader { get; }

/// <summary>
/// Gets a <see cref="StreamReader"/> object that can be used to read the contents
/// printed to <c>stderr</c> by a driver service process.
/// </summary>
public StreamReader? StandardErrorStreamReader { get; }
}
39 changes: 39 additions & 0 deletions dotnet/src/webdriver/DriverService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
using System.Net;
using System.Net.Http;
using System.Threading.Tasks;
using OpenQA.Selenium.Internal.Logging;

namespace OpenQA.Selenium;

Expand All @@ -37,6 +38,8 @@ public abstract class DriverService : ICommandServer
private bool isDisposed;
private Process? driverServiceProcess;

private static readonly ILogger _logger = Log.GetLogger(typeof(DriverService));

/// <summary>
/// Initializes a new instance of the <see cref="DriverService"/> class.
/// </summary>
Expand Down Expand Up @@ -151,6 +154,12 @@ public int ProcessId
/// </summary>
public string? DriverServicePath { get; set; }

/// <summary>
/// Collects the driver log output and writes it to the console. Defaults to <see langword="true"/>.
/// Internal variable to avoid using the stream when logs are sent to a file.
/// </summary>
protected bool WriteDriverLogToConsole { get; set; } = true;
Copy link
Member

Choose a reason for hiding this comment

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

I am pretty sure we don't need this property, despite its name is not clear. Just don't introduce it.

Copy link
Member Author

@diemol diemol Aug 4, 2025

Choose a reason for hiding this comment

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

In the case of sending GeckoDriver logs to a file, if we don't have a flag, even if it is internal, the streams get consumed by the console and the file. This ends up in an error because the console consumes the stream, and nothing gets sent to the file. That is why we need it.

Copy link
Member

Choose a reason for hiding this comment

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

Now, when we are listening logs as sync, finally we can remove this unnecessary property?


/// <summary>
/// Gets the command-line arguments for the driver service.
/// </summary>
Expand Down Expand Up @@ -243,6 +252,12 @@ public void Start()
this.driverServiceProcess.StartInfo.UseShellExecute = false;
this.driverServiceProcess.StartInfo.CreateNoWindow = this.HideCommandPromptWindow;

this.driverServiceProcess.StartInfo.RedirectStandardOutput = true;
this.driverServiceProcess.StartInfo.RedirectStandardError = true;

this.driverServiceProcess.OutputDataReceived += (s, e) => this.OnDriverProcessDataReceived(s, e, isError: false);
Copy link
Member

Choose a reason for hiding this comment

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

Please rename:
s => sender
e => args

this.driverServiceProcess.ErrorDataReceived += (s, e) => this.OnDriverProcessDataReceived(s, e, isError: true);

DriverProcessStartingEventArgs eventArgs = new DriverProcessStartingEventArgs(this.driverServiceProcess.StartInfo);
this.OnDriverProcessStarting(eventArgs);

Expand All @@ -251,6 +266,9 @@ public void Start()
DriverProcessStartedEventArgs processStartedEventArgs = new DriverProcessStartedEventArgs(this.driverServiceProcess);
this.OnDriverProcessStarted(processStartedEventArgs);

this.driverServiceProcess.BeginOutputReadLine();
this.driverServiceProcess.BeginErrorReadLine();

if (!serviceAvailable)
{
throw new WebDriverException($"Cannot start the driver service on {this.ServiceUrl}");
Expand Down Expand Up @@ -308,6 +326,27 @@ protected virtual void OnDriverProcessStarted(DriverProcessStartedEventArgs even
this.DriverProcessStarted?.Invoke(this, eventArgs);
}

/// <summary>
/// Handles the output and error data received from the driver process.
/// </summary>
/// <param name="sender">The sender of the event.</param>
/// <param name="args">The data received event arguments.</param>
/// <param name="isError">A value indicating whether the data received is from the error stream.</param>
protected virtual void OnDriverProcessDataReceived(object sender, DataReceivedEventArgs args, bool isError)
{
if (string.IsNullOrEmpty(args.Data))
return;

if (this.WriteDriverLogToConsole && !isError && _logger.IsEnabled(LogEventLevel.Info))
{
_logger.Info(args.Data);
Copy link
Member

Choose a reason for hiding this comment

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

We have never used Info level yet, it was preserved for really high level operations: like "starting browser" or "strating driver". Now the default level is Warn, and we can start using Info level.

But I think logs from driver service is something low-level, I propose always put them to Trace level.

}
if (this.WriteDriverLogToConsole && isError && _logger.IsEnabled(LogEventLevel.Error))
{
_logger.Error(args.Data);
Copy link
Member

Choose a reason for hiding this comment

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

From one point of view it might be reasonable. From another point of view Error level means "hey user, pay attention on this and fix it, or ask Selenium team to fix it!".

Error level is always revealed to end users, by default. I don't think user is able to fix errors came from driver services. I propose to redirect it as our Trace.

}
}

/// <summary>
/// Stops the DriverService.
/// </summary>
Expand Down
65 changes: 23 additions & 42 deletions dotnet/src/webdriver/Firefox/FirefoxDriverService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,11 @@

using OpenQA.Selenium.Internal;
using System;
using System.Diagnostics;
using System.Globalization;
using System.IO;
using System.Text;
using System.Threading.Tasks;
using OpenQA.Selenium.Internal.Logging;
Copy link
Member

Choose a reason for hiding this comment

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

Order namespaces (minor, up to you)


namespace OpenQA.Selenium.Firefox;

Expand All @@ -32,6 +33,7 @@ namespace OpenQA.Selenium.Firefox;
public sealed class FirefoxDriverService : DriverService
{
private const string DefaultFirefoxDriverServiceFileName = "geckodriver";
private static readonly ILogger _logger = Log.GetLogger(typeof(FirefoxDriverService));

/// <summary>
/// Process management fields for the log writer.
Expand Down Expand Up @@ -219,56 +221,55 @@ protected override string CommandLineArguments
}

/// <summary>
/// Handles the event when the driver service process is starting.
/// Called when the driver process is starting. This method sets up log file writing if a log path is specified.
/// </summary>
/// <param name="eventArgs">The event arguments containing information about the driver service process.</param>
/// <remarks>
/// This method initializes a log writer if a log path is specified and redirects output streams to capture logs.
/// This method initializes a log writer if a log path is specified.
/// </remarks>
protected override void OnDriverProcessStarting(DriverProcessStartingEventArgs eventArgs)
{
if (!string.IsNullOrEmpty(this.LogPath))
{
this.WriteDriverLogToConsole = false;
string? directory = Path.GetDirectoryName(this.LogPath);
if (!string.IsNullOrEmpty(directory) && !Directory.Exists(directory))
{
Directory.CreateDirectory(directory);
}

// Initialize the log writer
logWriter = new StreamWriter(this.LogPath, append: true) { AutoFlush = true };

// Configure process to redirect output
eventArgs.DriverServiceProcessStartInfo.RedirectStandardOutput = true;
eventArgs.DriverServiceProcessStartInfo.RedirectStandardError = true;
}

base.OnDriverProcessStarting(eventArgs);
}

/// <summary>
/// Handles the event when the driver process has started.
/// Handles the output and error data received from the driver process and sends it to the log writer if available.
/// </summary>
/// <param name="eventArgs">The event arguments containing information about the started driver process.</param>
/// <remarks>
/// This method reads the output and error streams asynchronously and writes them to the log file if available.
/// </remarks>
protected override void OnDriverProcessStarted(DriverProcessStartedEventArgs eventArgs)
/// <param name="sender">The sender of the event.</param>
/// <param name="args">The data received event arguments.</param>
/// <param name="isError">A value indicating whether the data received is from the error stream.</param>
protected override void OnDriverProcessDataReceived(object sender, DataReceivedEventArgs args, bool isError)
{
if (logWriter == null) return;
if (eventArgs.StandardOutputStreamReader != null)
if (string.IsNullOrEmpty(args.Data))
return;

if (!string.IsNullOrEmpty(this.LogPath))
{
_ = Task.Run(() => ReadStreamAsync(eventArgs.StandardOutputStreamReader));
if (logWriter != null)
{
logWriter.WriteLine(args.Data);
logWriter.Flush();
Copy link
Member

Choose a reason for hiding this comment

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

Flush can be removed, because AutoFlush is enabled.

Copy link
Member

Choose a reason for hiding this comment

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

Just to improve performance.

}
}

if (eventArgs.StandardErrorStreamReader != null)
else
{
_ = Task.Run(() => ReadStreamAsync(eventArgs.StandardErrorStreamReader));
base.OnDriverProcessDataReceived(sender, args, isError);
}

base.OnDriverProcessStarted(eventArgs);
}


/// <summary>
/// Disposes of the resources used by the <see cref="FirefoxDriverService"/> instance.
/// </summary>
Expand Down Expand Up @@ -368,24 +369,4 @@ private static string FirefoxDriverServiceFileName()

return fileName;
}

private async Task ReadStreamAsync(StreamReader reader)
{
try
{
string? line;
while ((line = await reader.ReadLineAsync()) != null)
{
if (logWriter != null)
{
logWriter.WriteLine($"{DateTime.Now:yyyy-MM-dd HH:mm:ss.fff} {line}");
}
}
}
catch (Exception ex)
{
// Log or handle the exception appropriately
System.Diagnostics.Debug.WriteLine($"Error reading stream: {ex.Message}");
}
}
}
70 changes: 66 additions & 4 deletions dotnet/test/firefox/FirefoxDriverServiceTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -17,25 +17,52 @@
// under the License.
// </copyright>

using System;
using System.Collections.Generic;
using NUnit.Framework;
using System.IO;
using System.Linq;
using OpenQA.Selenium.Internal.Logging;

namespace OpenQA.Selenium.Firefox;

[TestFixture]
public class FirefoxDriverServiceTest : DriverTestFixture
public class FirefoxDriverServiceTest
{
private TestLogHandler testLogHandler;

private void ResetGlobalLog()
{
Log.SetLevel(LogEventLevel.Info);
Log.Handlers.Clear().Handlers.Add(new TextWriterHandler(Console.Error));
}

[SetUp]
public void SetUp()
{
ResetGlobalLog();

testLogHandler = new TestLogHandler();
}

[TearDown]
public void TearDown()
{
ResetGlobalLog();
}

[Test]
public void ShouldRedirectGeckoDriverLogsToFile()
{
FirefoxOptions options = new FirefoxOptions();
string logPath = Path.GetTempFileName();
options.LogLevel = FirefoxDriverLogLevel.Trace;
options.LogLevel = FirefoxDriverLogLevel.Info;

FirefoxDriverService service = FirefoxDriverService.CreateDefaultService();
service.LogPath = logPath;

IWebDriver driver2 = new FirefoxDriver(service, options);
IWebDriver firefoxDriver = new FirefoxDriver(service, options);
firefoxDriver.Quit();

try
{
Expand All @@ -45,9 +72,44 @@ public void ShouldRedirectGeckoDriverLogsToFile()
}
finally
{
driver2.Quit();
File.Delete(logPath);
}
}

[Test]
public void ShouldRedirectGeckoDriverLogsToConsole()
Copy link
Member

@nvborisenko nvborisenko Aug 6, 2025

Choose a reason for hiding this comment

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

Specific driver tests are ignored. It will be nice to reincarnate them here: #15536

Read and resolve this thread.

{
Log.SetLevel(LogEventLevel.Info).Handlers.Add(testLogHandler);
FirefoxOptions options = new FirefoxOptions();
options.LogLevel = FirefoxDriverLogLevel.Info;

FirefoxDriverService service = FirefoxDriverService.CreateDefaultService();

IWebDriver firefoxDriver = new FirefoxDriver(service, options);

try
{
Assert.That(testLogHandler.Events, Has.Count.AtLeast(1));
Assert.That(testLogHandler.Events.Any(e => e.Message.Contains("geckodriver")), Is.True);
}
finally
{
firefoxDriver.Quit();
}
}
}

class TestLogHandler : ILogHandler
{
public ILogHandler Clone()
{
return this;
}

public void Handle(LogEvent logEvent)
{
Events.Add(logEvent);
}

public IList<LogEvent> Events { get; internal set; } = new List<LogEvent>();
}
Loading