[dotnet] Start ChromeDriver asynchronously#17112
[dotnet] Start ChromeDriver asynchronously#17112nvborisenko wants to merge 4 commits intoSeleniumHQ:trunkfrom
Conversation
PR TypeEnhancement Description
File Walkthrough
|
PR Compliance Guide 🔍Below is a summary of compliance checks for this PR:
Compliance status legend🟢 - Fully Compliant🟡 - Partial Compliant 🔴 - Not Compliant ⚪ - Requires Further Human Verification 🏷️ - Compliance label |
|||||||||||||||||||||||||||||||||
PR Code Suggestions ✨Explore these optional code suggestions:
|
|||||||||||
There was a problem hiding this comment.
Pull request overview
This PR adds async driver initialization to the .NET bindings, enabling ChromeDriver (and related Chromium plumbing) to be started via an awaitable API while preserving existing synchronous constructors.
Changes:
- Refactors
WebDriversession creation to support optional automatic session start and introducesStartSessionAsync. - Adds
ChromeDriver.StartAsync(...)factory methods for asynchronous driver startup. - Exposes async service/executor creation hooks in
ChromiumDriverto support async startup flows.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| dotnet/src/webdriver/WebDriver.cs | Adds optional auto-start session behavior and introduces async session-start implementation. |
| dotnet/src/webdriver/Chromium/ChromiumDriver.cs | Adds constructor overload to control auto session start and exposes async executor generation for derived drivers. |
| dotnet/src/webdriver/Chrome/ChromeDriver.cs | Introduces StartAsync APIs and a constructor overload to support async startup without auto session creation. |
| } | ||
|
|
||
| ChromeDriverService service = ChromeDriverService.CreateDefaultService(); | ||
| ICommandExecutor executor = await GenerateDriverServiceCommandExecutorAsync(service, options, DefaultCommandTimeout).ConfigureAwait(false); |
There was a problem hiding this comment.
StartAsync(ChromeOptions) creates a ChromeDriverService before calling GenerateDriverServiceCommandExecutorAsync(...), but if that call throws (e.g., driver/browser discovery fails before the internal try/catch that disposes the service), the newly created service instance is never disposed. Wrap executor creation in a try/catch and dispose the service on failure (prefer DisposeAsync if appropriate) to avoid leaking resources when startup fails early.
| ICommandExecutor executor = await GenerateDriverServiceCommandExecutorAsync(service, options, DefaultCommandTimeout).ConfigureAwait(false); | |
| ICommandExecutor executor; | |
| try | |
| { | |
| executor = await GenerateDriverServiceCommandExecutorAsync(service, options, DefaultCommandTimeout).ConfigureAwait(false); | |
| } | |
| catch | |
| { | |
| service.Dispose(); | |
| throw; | |
| } |
| /// <summary> | ||
| /// Asynchronously creates and starts a new instance of the <see cref="ChromeDriver"/> class with default options. | ||
| /// </summary> | ||
| /// <returns>A task that represents the asynchronous operation. The task result contains the initialized <see cref="ChromeDriver"/>.</returns> | ||
| public static Task<ChromeDriver> StartAsync() | ||
| { | ||
| return StartAsync(new ChromeOptions()); | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Asynchronously creates and starts a new instance of the <see cref="ChromeDriver"/> class using the specified options. | ||
| /// </summary> | ||
| /// <param name="options">The <see cref="ChromeOptions"/> to be used with the Chrome driver.</param> | ||
| /// <returns>A task that represents the asynchronous operation. The task result contains the initialized <see cref="ChromeDriver"/>.</returns> | ||
| /// <exception cref="ArgumentNullException">If <paramref name="options"/> is <see langword="null"/>.</exception> | ||
| public static async Task<ChromeDriver> StartAsync(ChromeOptions options) |
There was a problem hiding this comment.
The new ChromeDriver.StartAsync(...) entry point is user-facing behavior, but there are no tests validating it (successful startup path and failure/disposal behavior). Please add coverage in the existing .NET test suite (e.g., under dotnet/test/chrome or dotnet/test/common) to ensure StartAsync actually creates a usable session and that resources are cleaned up on exceptions.
|
Are you going to flag the standard constructor [Obsolete] and push everyone towards the new static factory async method? |
|
Not sure how it should happen. My vote is to obsolete in v5 and remove in v6. Keep eyes on #14067 |
| { | ||
| if (options is null) | ||
| { | ||
| throw new ArgumentNullException(nameof(options), "Chrome options must not be null"); |
There was a problem hiding this comment.
| throw new ArgumentNullException(nameof(options), "Chrome options must not be null"); | |
| throw new ArgumentNullException(nameof(options)); |
I would remove the custom exception message. The default one is fine.
|
Generally, looks great. I would additionally like to have a full public static async Task<ChromeDriver> StartAsync(
ChromeDriverService service,
ChromeOptions options,
TimeSpan commandTimeout,
CancellationToken cancellationToken = default); |
|
Some users (at least me) anyway might still need |
|
Should be covered by #17115, hopefully we will do something interesting there. |
🔗 Related Issues
Contributes to #17086 and #14067
💥 What does this PR do?
Introduces asynchronous initialization support for the ChromeDriver and ChromiumDriver classes, refactors driver session startup logic to allow optional automatic session creation, and improves code clarity and safety with better nullability handling and modern C# syntax. The changes also add comprehensive documentation for new and existing APIs.
💡 Additional Considerations
Only overload which takes
ChromeOptions. Others will come soon?🔄 Types of changes