BugFix - 39399 - Unable To Launch Firefox From Playwright#3747
Conversation
RC: If the firefox browser is not installed in the user's machine then Playwright is unable to launch it. Fix: If the browser executable is not found, then execute Playwright's install command to download and install the relevant browser.
WalkthroughThe recent updates enhance the robustness of the Playwright driver and the Agent operations in the Ginger framework. Key improvements include better error handling for browser executable issues and enhanced logging for driver startup errors. These changes aim to improve reliability and debugging capabilities. Changes
Sequence Diagram(s)sequenceDiagram
participant AgentOperations
participant Driver
participant Logger
AgentOperations->>Driver: StartDriver()
alt Driver Throws Exception
Driver-->>AgentOperations: Exception
AgentOperations->>Logger: Log Error
else Driver Starts Successfully
Driver-->>AgentOperations: Success
end
sequenceDiagram
participant PlaywrightDriver
participant Playwright
participant System
PlaywrightDriver->>Playwright: LaunchBrowserWithInstallationAsync()
alt Browser Executable Not Found
PlaywrightDriver->>System: ExecutePlaywrightInstallationCommand()
end
Playwright-->>PlaywrightDriver: Browser Instance
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Playwright/PlaywrightDriver.cs (3 hunks)
Additional context used
GitHub Check: Codacy Static Code Analysis
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Playwright/PlaywrightDriver.cs
[warning] 156-156: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Playwright/PlaywrightDriver.cs#L156
'System.Exception' should not be thrown by user code.
Additional comments not posted (5)
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Playwright/PlaywrightDriver.cs (5)
14-20: Added necessary using directives to support new functionality.
27-28: Introduced a constant and a static field to manage browser executable errors and synchronize Playwright installation commands.
48-48: RenamedLaunchBrowserAsynctoLaunchBrowserWithInstallationAsyncto reflect the new functionality that includes handling installation.
91-111: RefactoredLaunchBrowserWithInstallationAsyncto handle cases where the browser executable is not found. This method now attempts to install the required browser before retrying the launch.
135-162: AddedExecutePlaywrightInstallationCommandto manage the installation of browsers. This method uses a switch statement to determine the browser type and executes the installation command accordingly. It also properly handles synchronization usingAutoResetEvent.Tools
GitHub Check: Codacy Static Code Analysis
[warning] 156-156: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Playwright/PlaywrightDriver.cs#L156
'System.Exception' should not be thrown by user code.
| int exitCode = Program.Main(new[] { "install", browserTypeString }); | ||
| if (exitCode != 0) | ||
| { | ||
| throw new Exception($"Error occurred while executing playwright installation command, exited with code {exitCode}"); |
There was a problem hiding this comment.
Avoid throwing generic exceptions.
Consider throwing a more specific exception type or creating a custom exception class to provide more detailed error information.
- throw new Exception($"Error occurred while executing playwright installation command, exited with code {exitCode}");
+ throw new PlaywrightInstallationException($"Error occurred while executing playwright installation command, exited with code {exitCode}");Committable suggestion was skipped due to low confidence.
Tools
GitHub Check: Codacy Static Code Analysis
[warning] 156-156: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Playwright/PlaywrightDriver.cs#L156
'System.Exception' should not be thrown by user code.
IamHarsimranjeet
changed the title
removed sync event for installation command since, playwright handles it internally.
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Playwright/PlaywrightDriver.cs (3 hunks)
- Ginger/GingerCoreNET/RunLib/AgentOperations.cs (1 hunks)
Additional context used
GitHub Check: Codacy Static Code Analysis
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Playwright/PlaywrightDriver.cs
[warning] 147-147: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Playwright/PlaywrightDriver.cs#L147
'System.Exception' should not be thrown by user code.
Additional comments not posted (3)
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Playwright/PlaywrightDriver.cs (2)
21-21: Introduced a constant for error messages which is a good practice for maintainability.
84-104: Refactored method to handle browser installation if the executable is not found. This improves robustness and user experience by attempting an automatic fix.Ginger/GingerCoreNET/RunLib/AgentOperations.cs (1)
202-209: Enhanced error handling in theStartDrivermethod by logging exceptions. This ensures that all driver start issues are properly reported, improving debuggability.
| private static void ExecutePlaywrightInstallationCommand(WebBrowserType browserType) | ||
| { | ||
| string browserTypeString; | ||
| switch (browserType) | ||
| { | ||
| case WebBrowserType.Chrome: | ||
| case WebBrowserType.Edge: | ||
| browserTypeString = Microsoft.Playwright.BrowserType.Chromium; | ||
| break; | ||
| case WebBrowserType.FireFox: | ||
| browserTypeString = Microsoft.Playwright.BrowserType.Firefox; | ||
| break; | ||
| default: | ||
| throw new ArgumentException($"Unknown browser type '{browserType}'"); | ||
| } | ||
|
|
||
| int exitCode = Program.Main(new[] { "install", browserTypeString }); | ||
| if (exitCode != 0) | ||
| { | ||
| throw new Exception($"Error occurred while executing playwright installation command, exited with code {exitCode}"); | ||
| } |
There was a problem hiding this comment.
Replace generic exception with a more specific type to improve error handling.
- throw new Exception($"Error occurred while executing playwright installation command, exited with code {exitCode}");
+ throw new PlaywrightInstallationException($"Error occurred while executing playwright installation, exited with code {exitCode}");Committable suggestion was skipped due to low confidence.
Tools
GitHub Check: Codacy Static Code Analysis
[warning] 147-147: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Playwright/PlaywrightDriver.cs#L147
'System.Exception' should not be thrown by user code.
2 participants
Thank you for your contribution.
Before submitting this PR, please make sure:
Summary by CodeRabbit
New Features
Bug Fixes