Skip to content

[py] Add --enable-chrome-logs to chrome service args to inherit browser i/o streams#16202

Merged
cgoldberg merged 5 commits into
SeleniumHQ:trunkfrom
cgoldberg:py-enable-chrome-logs-flag
Sep 30, 2025
Merged

[py] Add --enable-chrome-logs to chrome service args to inherit browser i/o streams#16202
cgoldberg merged 5 commits into
SeleniumHQ:trunkfrom
cgoldberg:py-enable-chrome-logs-flag

Conversation

@cgoldberg

@cgoldberg cgoldberg commented Aug 18, 2025 •
edited by qodo-code-review Bot
Loading

Copy link
Copy Markdown
Member

User description

🔗 Related Issues

implements #16201 for Python
fixes #16117
fixes #13095

💥 What does this PR do?

This PR adds the --enable-chrome-logs argument when starting chromedriver. This allows it to inherit the i/o streams from the browser process. Without this, we have no way to suppress or redirect browser i/o streams, which results in unwanted logging going to the user's console.

🔧 Implementation Notes

I added this to the chrome Service class rather than the chromium Service class because I am not sure if this is also supported by Edge, or is only relevant to Chrome.

💡 Additional Considerations

We should test to make sure this doesn't screw up any other logging features.

🔄 Types of changes

  • Bug fix (backwards compatible)

PR Type

Bug fix

Description

  • Add --enable-chrome-logs flag to Chrome service arguments

  • Enable browser I/O stream inheritance to suppress unwanted console logging

  • Override command_line_args() method in Chrome service class

Diagram Walkthrough

flowchart LR
  A["Chrome Service"] --> B["command_line_args()"] 
  B --> C["--enable-chrome-logs flag"]
  C --> D["Browser I/O streams inherited"]
  D --> E["Suppressed console logging"]
Loading

File Walkthrough
Relevant files
Bug fix
service.py
Add Chrome logs flag to service arguments                               

py/selenium/webdriver/chrome/service.py

  • Override command_line_args() method to include --enable-chrome-logs
    flag
  • Add flag before port and service arguments in command line
  • Enable browser process I/O stream inheritance
 +3/-0     

@selenium-ci selenium-ci added the C-py Python Bindings label Aug 18, 2025
@cgoldberg cgoldberg mentioned this pull request Aug 18, 2025
Open
@qodo-code-review

qodo-code-review Bot commented Aug 18, 2025

Copy link
Copy Markdown
Contributor

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Backward Compatibility

Adding '--enable-chrome-logs' unconditionally changes default behavior and may affect users relying on previous ChromeDriver/Chrome logging behavior; verify that this flag exists across supported Chrome/Driver versions and does not regress logging/CI stderr handling.

 
def command_line_args(self) -> list[str]:
    return ["--enable-chrome-logs", f"--port={self.port}"] + self._service_args
Argument Ordering

Ensure that placing '--enable-chrome-logs' before port and appending user-provided service args does not lead to duplicate or conflicting flags (e.g., user also passing '--enable-chrome-logs' or a different '--port'); consider de-duping or precedence rules.

 
def command_line_args(self) -> list[str]:
    return ["--enable-chrome-logs", f"--port={self.port}"] + self._service_args

@qodo-code-review

qodo-code-review Bot commented Aug 18, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Ensure port precedence and None safety

Place the port argument after user-specified service_args to avoid being
overridden by a user-provided --port in _service_args. Also, guard against None
to prevent a TypeError when no service args are provided.

py/selenium/webdriver/chrome/service.py [57-58]

 def command_line_args(self) -> list[str]:
-    return ["--enable-chrome-logs", f"--port={self.port}"] + self._service_args
+    args = ["--enable-chrome-logs"]
+    user_args = self._service_args or []
+    # Ensure driver-required port comes last to take precedence
+    args.extend(user_args)
+    args.append(f"--port={self.port}")
+    return args
  • Apply / Chat
Suggestion importance[1-10]: 9

__

Why: The suggestion correctly identifies a critical issue where a user-provided --port in service_args could override the port managed by the service, leading to connection failures. Moving the port argument to the end ensures its precedence.

High
  • Update

@cgoldberg cgoldberg mentioned this pull request Aug 18, 2025
Closed
@cgoldberg cgoldberg marked this pull request as draft August 18, 2025 13:58
@cgoldberg

cgoldberg commented Aug 18, 2025

Copy link
Copy Markdown
Member Author

Marking this as a draft. It seems to break things if you launch with --enable-chrome-logs but don't redirect i/o.

@cgoldberg cgoldberg marked this pull request as ready for review August 28, 2025 19:06
@qodo-code-review

qodo-code-review Bot commented Aug 28, 2025

Copy link
Copy Markdown
Contributor

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Compatibility Risk

Verify that chromedriver versions used in supported environments recognize the --enable-chrome-logs switch; older versions may not accept it and could fail to start or ignore it. Consider feature-detection or graceful fallback if the flag is unrecognized.

 
def command_line_args(self) -> list[str]:
    return ["--enable-chrome-logs", f"--port={self.port}"] + self._service_args
Arg Ordering

Confirm that placing --enable-chrome-logs before --port and ahead of other service args matches chromedriver expectations and does not conflict with flags users pass via service_args (e.g., duplicates or overrides).

 
def command_line_args(self) -> list[str]:
    return ["--enable-chrome-logs", f"--port={self.port}"] + self._service_args
Configurability

Consider allowing users to opt out of --enable-chrome-logs (e.g., via a constructor option) to avoid changing default behavior for consumers who rely on current logging semantics.

 
def command_line_args(self) -> list[str]:
    return ["--enable-chrome-logs", f"--port={self.port}"] + self._service_args

@qodo-code-review

qodo-code-review Bot commented Aug 28, 2025

Copy link
Copy Markdown
Contributor

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Preserve base CLI arguments

Reuse the base implementation to avoid dropping any default/required arguments
and to preserve their ordering. Also switch the return annotation to
Sequence[str] for broader Python version compatibility. Ensure the chrome logs
flag is added only once.

py/selenium/webdriver/chrome/service.py [57-58]

-def command_line_args(self) -> list[str]:
-    return ["--enable-chrome-logs", f"--port={self.port}"] + self._service_args
+def command_line_args(self) -> Sequence[str]:
+    base_args = list(super().command_line_args())
+    if "--enable-chrome-logs" not in base_args:
+        base_args.insert(0, "--enable-chrome-logs")
+    return base_args
  • Apply / Chat
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly points out that overriding command_line_args without calling the parent implementation is brittle. Reusing the base method via super() makes the code more maintainable and robust against future changes in the parent ChromiumService class.

Medium
  • More

@cgoldberg cgoldberg merged commit 9a4bbcc into SeleniumHQ:trunk Sep 30, 2025
16 checks passed
@cgoldberg cgoldberg deleted the py-enable-chrome-logs-flag branch September 30, 2025 13:40
@cgoldberg cgoldberg mentioned this pull request May 5, 2026
Merged
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

C-py Python Bindings Review effort 2/5

Projects

None yet

Milestone

No milestone

2 participants

@cgoldberg @selenium-ci