[grid][java]: relay service can set protocol version in fetching status

[VietND96]

User description

Thanks for contributing to Selenium!
A PR well described will help maintainers to quickly review and merge it

Before submitting your PR, please check our contributing guidelines.
Avoid large PRs, help reviewers by making them as simple and short as possible.

Description

Motivation and Context

Fix: #13481

15:48:49.620 WARN [RelaySessionFactory.isServiceUp] - Error checking service status http://localhost:4723/status. java.io.IOException: HTTP/1.1 header parser received no bytes
15:48:49.621 ERROR [NodeServer$1.lambda$start$1] - Node is not alive: http://192.168.1.144:5557 is DOWN

Pass the protocol version to the HttpClient config before executing the request in method isServiceUp()
Required the CLI config --service-protocol-version or protocol-version in TOML config
The config accepts values following enum. For example: HTTP/1.1 (or HTTP_1_1), HTTP/2 (or HTTP_2)

TOML config looks like

[server]
port = 5555

[node]
detect-drivers = false

[relay]
url = "http://localhost:4723"
status-endpoint = "/status"
protocol-version = "HTTP/1.1"
configs = [
    "1", "{\"platformName\": \"android\", \"appium:platformVersion\": \"10\", \"appium:deviceName\": \"Samsung SM-G988\", \"appium:automationName\": \"uiautomator2\"}"
]

For testing it works is mentioned in #13481 (comment)

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist

• I have read the contributing document.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have added tests to cover my changes.
• All new and existing tests passed.

---

Type

Enhancement

---

Description

• Added support for specifying the HTTP protocol version in Selenium Grid's relay service.
• Introduced new configuration parameters and command-line options to set the desired HTTP protocol version.
• Enhanced the RelaySessionFactory and RelayOptions classes to utilize the specified protocol version when creating HttpClient instances.
• Added comprehensive tests to ensure the new configuration is parsed correctly and handles errors as expected.

---

Changes walkthrough

	Relevant files
Enhancement	RelayFlags.java Add service protocol version configuration in RelayFlags  java/src/org/openqa/selenium/grid/node/relay/RelayFlags.java Added a new command-line parameter and configuration value for specifying the HTTP protocol version when communicating with the service status endpoint. +8/-0      RelayOptions.java Implement service protocol version handling in RelayOptions java/src/org/openqa/selenium/grid/node/relay/RelayOptions.java Added method to fetch and validate the service protocol version from configuration. Enhanced isServiceUp method to use the specified protocol version when creating the HttpClient. +17/-0    RelaySessionFactory.java Support protocol version in RelaySessionFactory                    java/src/org/openqa/selenium/grid/node/relay/RelaySessionFactory.java Added handling for the service protocol version in the session factory constructor. Modified isServiceUp method to configure the HttpClient with the specified protocol version. +10/-1
Tests	RelayOptionsTest.java Add tests for service protocol version parsing in RelayOptions java/test/org/openqa/selenium/grid/node/relay/RelayOptionsTest.java Added tests to verify the correct parsing and handling of the service protocol version. Included tests to ensure proper exception handling for unsupported protocol versions. +48/-0

---

✨ PR-Agent usage:
Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

[codiumai-pr-agent-pro]

PR Description updated to latest commit (e89e5bd)

• Copy walkthrough table to "Files Changed" Tab

[codiumai-pr-agent-pro]

PR Review

⏱️ Estimated effort to review [1-5]	3, because the PR involves changes across multiple files including configuration, option handling, and session management. The logic to handle protocol versions and its integration requires careful review to ensure compatibility and correctness.
🧪 Relevant tests	Yes
🔍 Possible issues	Possible Bug: The method getServiceProtocolVersion in RelayOptions.java converts the protocol version string to an enum and back to a string. This could lead to unexpected behavior or errors if the enum does not have a corresponding value for the user's input, despite the catch for IllegalArgumentException.
	Configuration Error Handling: The exception handling for invalid protocol versions could be more user-friendly by suggesting valid options or providing more context in the error message.
🔒 Security concerns	No

---

✨ Review tool usage guide:

---

Overview:
The review tool scans the PR code changes, and generates a PR review which includes several types of feedbacks, such as possible PR issues, security threats and relevant test in the PR. More feedbacks can be added by configuring the tool.

The tool can be triggered automatically every time a new PR is opened, or can be invoked manually by commenting on any PR.

• When commenting, to edit configurations related to the review tool (pr_reviewer section), use the following template:

/review --pr_reviewer.some_config1=... --pr_reviewer.some_config2=...

• With a configuration file, use the following template:

[pr_reviewer]
some_config1=...
some_config2=...

See the review usage page for a comprehensive guide on using this tool.

[codiumai-pr-agent-pro]

PR Code Suggestions

Category	Suggestions
Maintainability	Improve the normalization and validation of protocol versions. Replace the string replacements and toUpperCase with a more robust method of normalizing and validating the protocol version. Consider creating a utility method that handles the normalization and validation of protocol versions to ensure consistency and reduce code duplication. java/src/org/openqa/selenium/grid/node/relay/RelayOptions.java [113] -protocolVersion = protocolVersion.toUpperCase().replaceAll("/", "_").replaceAll("\\.", "_"); +protocolVersion = normalizeProtocolVersion(protocolVersion);
Error handling	Add error handling for invalid protocol version inputs. Handle the case where Version.valueOf(protocolVersion) throws an IllegalArgumentException by logging the invalid version and providing a default version or rethrowing a more informative exception. java/src/org/openqa/selenium/grid/node/relay/RelayOptions.java [116] -return Version.valueOf(protocolVersion).toString(); +try { + return Version.valueOf(protocolVersion).toString(); +} catch (IllegalArgumentException e) { + LOG.error("Invalid protocol version: " + protocolVersion, e); + throw new ConfigException("Invalid protocol version provided: " + protocolVersion, e); +}
Bug prevention	Check for non-empty protocol version before setting it in the client configuration. Ensure that the serviceProtocolVersion is not null or empty before setting it in the ClientConfig. This prevents potential issues with client configuration when an invalid or empty version is used. java/src/org/openqa/selenium/grid/node/relay/RelaySessionFactory.java [219] -clientConfig = clientConfig.version(serviceProtocolVersion); +if (!serviceProtocolVersion.isEmpty()) { + clientConfig = clientConfig.version(serviceProtocolVersion); +}
Enhancement	Refactor method to return an Optional for better error handling. Refactor the getServiceProtocolVersion method to return an Optional instead of throwing an exception directly. This allows the caller to handle the absence of a valid protocol version more gracefully. java/src/org/openqa/selenium/grid/node/relay/RelayOptions.java [116] -return Version.valueOf(protocolVersion).toString(); +try { + return Optional.of(Version.valueOf(protocolVersion).toString()); +} catch (IllegalArgumentException e) { + return Optional.empty(); +}
Test enhancement	Add a unit test for unsupported protocol versions. Add a unit test to cover the scenario where an unsupported protocol version is provided, ensuring that the system behaves as expected in such cases. java/src/org/openqa/selenium/grid/node/relay/RelayOptions.java [141-143] assertThatExceptionOfType(ConfigException.class) .isThrownBy(() -> { relayOptions.getServiceProtocolVersion(); - }); + }).withMessageContaining("Unsupported protocol version");

---

✨ Improve tool usage guide:

---

Overview:
The improve tool scans the PR code changes, and automatically generates suggestions for improving the PR code. The tool can be triggered automatically every time a new PR is opened, or can be invoked manually by commenting on a PR.

• When commenting, to edit configurations related to the improve tool (pr_code_suggestions section), use the following template:

/improve --pr_code_suggestions.some_config1=... --pr_code_suggestions.some_config2=...

• With a configuration file, use the following template:

[pr_code_suggestions]
some_config1=...
some_config2=...

See the improve usage page for a comprehensive guide on using this tool.

[VietND96]

@diemol, can you review this and include it in the 4.20.0 release?

[diemol]

Thank you, @VietND96!

Can you check the code suggestions?

[VietND96]

I fixed some points

Ensure that the serviceProtocolVersion is not null or empty before setting it in the
ClientConfig. This prevents potential issues with client configuration when an invalid or
empty version is used.

java/src/org/openqa/selenium/grid/node/relay/RelaySessionFactory.java [219]

This one already guarded in part of constructor, so can ignore this.serviceProtocolVersion = Require.nonNull("Service protocol version", serviceProtocolVersion);

[diemol]

Thank you, @VietND96!