Added BrokenLinkChecker utility to verify broken links #15543

OmkarGharat · 2025-04-01T07:00:58Z

User description

Contributing to Selenium!

Motivation and Context

In many Selenium projects, ensuring that the web application links are functioning properly is a common requirement. This change introduces a utility method to check whether a link is broken by sending an HTTP HEAD request, which helps avoid loading unnecessary data. The method uses the response code from the server to determine whether the link is working or broken (response code >= 400).

This improvement will be useful for anyone automating tests and verifying the functionality of links on a webpage without needing to load the entire content of the target URL.

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](https://github.com/SeleniumHQ/selenium/blob/trunk/CONTRIBUTING.md) 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.

This pull request introduces the BrokenLinkChecker class with methods to verify whether a link is broken by using a HEAD request instead of loading the entire content of the link. The class supports checking both individual web elements (WebElement) and URLs directly. The isBroken() method handles both cases by checking the href attribute of a web element and sending a HEAD request to the URL, and returns a boolean indicating if the link is broken (response code >= 400).

PR Type

Enhancement, Tests

Description

Introduced BrokenLinkChecker utility class for link verification.
Added isBroken(WebElement) method to check links in web elements.
Added isBroken(String) method to validate URLs using HTTP HEAD requests.
Handles invalid URLs and connection errors gracefully.

Changes walkthrough 📝

Relevant files

Enhancement

BrokenLinkChecker.java Introduced `BrokenLinkChecker` utility class for link validation java/src/org/openqa/selenium/support/BrokenLinkChecker.java Added `BrokenLinkChecker` utility class to check broken links. Implemented `isBroken(WebElement)` to validate links in web elements. Implemented `isBroken(String)` to validate URLs using HTTP `HEAD` requests. Included error handling for invalid URLs and connection issues.	+41/-0

Need help?
Type /help how to ... in the comments thread for any questions about Qodo Merge usage.
Check out the documentation for more information.

qodo-merge-pro · 2025-04-01T07:01:18Z

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review Error Handling The method returns false for empty href attributes but doesn't handle relative URLs, which could lead to invalid URI exceptions. Consider adding support for relative URLs by checking if the URL is absolute before processing. String url = element.getAttribute("href"); if (url == null \|\| url.trim().isEmpty()) { return false; } return isBroken(url.trim()); Logging Approach Using System.err.println for error logging instead of a proper logging framework. Consider using SLF4J or java.util.logging for better integration with Selenium's logging system. System.err.println("Error checking link: " + linkURL + " → " + e.getMessage()); Exception Handling The method catches only IOException but not other potential exceptions like IllegalArgumentException from URI.create(). This could cause unexpected failures when processing malformed URLs. try { URL url = URI.create(linkURL).toURL(); HttpURLConnection connection = (HttpURLConnection) url.openConnection(); connection.setRequestMethod("HEAD"); connection.setConnectTimeout(5000); // 5 sec timeout connection.setReadTimeout(5000); connection.connect(); int responseCode = connection.getResponseCode(); connection.disconnect(); return responseCode >= 400; } catch (IOException e) { System.err.println("Error checking link: " + linkURL + " → " + e.getMessage()); return true; }

qodo-merge-pro · 2025-04-01T07:01:48Z

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	✅ ~~Handle malformed URL exceptions~~ Suggestion Impact: The commit implemented the suggestion by adding IllegalArgumentException to the catch block in the isBroken method, exactly as suggested. The commit also added additional improvements like using a logger instead of System.err.println, but the core suggestion of handling the IllegalArgumentException was implemented. code diff: + } catch (IOException \| IllegalArgumentException e) { + // Using SLF4J logger for logging the exception + logger.warn("Error checking link: " + linkURL, e); // Log the exception with warning level + return true; // Return true if any exception occurs (link is considered broken) The method doesn't handle `IllegalArgumentException` that can be thrown by `URI.create()` when the URL is malformed. This could cause unexpected crashes. java/src/org/openqa/selenium/support/BrokenLinkChecker.java [23-39] public static boolean isBroken(String linkURL) { try { URL url = URI.create(linkURL).toURL(); HttpURLConnection connection = (HttpURLConnection) url.openConnection(); connection.setRequestMethod("HEAD"); connection.setConnectTimeout(5000); // 5 sec timeout connection.setReadTimeout(5000); connection.connect(); int responseCode = connection.getResponseCode(); connection.disconnect(); return responseCode >= 400; - } catch (IOException e) { + } catch (IOException \| IllegalArgumentException e) { System.err.println("Error checking link: " + linkURL + " → " + e.getMessage()); return true; } } `[To ensure code accuracy, apply this suggestion manually]` Suggestion importance[1-10]: 9 __ Why: This suggestion addresses a critical exception handling gap. URI.create() can throw IllegalArgumentException for malformed URLs, which would cause the method to crash unexpectedly. Adding this exception to the catch block significantly improves the robustness of the code.	High
Possible issue	✅ ~~Fix empty link validation~~ Suggestion Impact: The commit implemented the suggestion by changing the return value from false to true when a URL is null or empty, and added a comment explaining that empty or null URLs are treated as broken code diff: if (url == null \|\| url.trim().isEmpty()) { - return false; + return true; // Treat empty or null URLs as broken The method incorrectly returns `false` for empty or null URLs. Empty links should be considered broken since they don't point to valid resources. java/src/org/openqa/selenium/support/BrokenLinkChecker.java [15-21] public static boolean isBroken(WebElement element) { String url = element.getAttribute("href"); if (url == null \|\| url.trim().isEmpty()) { - return false; + return true; } return isBroken(url.trim()); } `[Suggestion has been applied]` Suggestion importance[1-10]: 8 __ Why: This suggestion correctly identifies a logical issue in the code. Empty or null href attributes should be considered broken links since they don't point to valid resources. Changing the return value from false to true for these cases is a meaningful improvement to the utility's functionality.	Medium
Learned best practice	✅ ~~Add parameter validation to prevent null pointer exceptions by checking method parameters before use~~ Suggestion Impact: The commit implemented the exact null checks suggested for both the element and linkURL parameters, adding the same IllegalArgumentException throws with the suggested error messages. code diff: + if (element == null) { + throw new IllegalArgumentException("Element cannot be null"); // Validate element + } + String url = element.getAttribute("href"); if (url == null \|\| url.trim().isEmpty()) { - return false; + return true; // Treat empty or null URLs as broken } return isBroken(url.trim()); } public static boolean isBroken(String linkURL) { + if (linkURL == null) { + throw new IllegalArgumentException("Link URL cannot be null"); // Validate linkURL + } Add null checks for the `element` and `linkURL` parameters before using them to prevent NullPointerExceptions. The current implementation checks if the URL string is null but doesn't validate that the input parameters themselves are not null. java/src/org/openqa/selenium/support/BrokenLinkChecker.java [15-30] public static boolean isBroken(WebElement element) { + if (element == null) { + throw new IllegalArgumentException("Element cannot be null"); + } String url = element.getAttribute("href"); if (url == null \|\| url.trim().isEmpty()) { return false; } return isBroken(url.trim()); } public static boolean isBroken(String linkURL) { + if (linkURL == null) { + throw new IllegalArgumentException("Link URL cannot be null"); + } try { URL url = URI.create(linkURL).toURL(); HttpURLConnection connection = (HttpURLConnection) url.openConnection(); connection.setRequestMethod("HEAD"); ... `[To ensure code accuracy, apply this suggestion manually]` Suggestion importance[1-10]: 6	Low
General	✅ ~~Improve error logging~~ Suggestion Impact: The suggestion was implemented by adding SLF4J logger. The commit added a logger instance at the class level and replaced System.err.println with logger.warn() as suggested. code diff: +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; public final class BrokenLinkChecker { + + private static final Logger logger = LoggerFactory.getLogger(BrokenLinkChecker.class); // Create a logger instance private BrokenLinkChecker() { // Utility class - prevent instantiation } public static boolean isBroken(WebElement element) { + if (element == null) { + throw new IllegalArgumentException("Element cannot be null"); // Validate element + } + String url = element.getAttribute("href"); if (url == null \|\| url.trim().isEmpty()) { - return false; + return true; // Treat empty or null URLs as broken } return isBroken(url.trim()); } public static boolean isBroken(String linkURL) { + if (linkURL == null) { + throw new IllegalArgumentException("Link URL cannot be null"); // Validate linkURL + } + try { - URL url = URI.create(linkURL).toURL(); + URL url = URI.create(linkURL).toURL(); // This could throw IllegalArgumentException HttpURLConnection connection = (HttpURLConnection) url.openConnection(); connection.setRequestMethod("HEAD"); connection.setConnectTimeout(5000); // 5 sec timeout @@ -32,10 +44,11 @@ int responseCode = connection.getResponseCode(); connection.disconnect(); - return responseCode >= 400; - } catch (IOException e) { - System.err.println("Error checking link: " + linkURL + " → " + e.getMessage()); - return true; + return responseCode >= 400; // Broken link if response code is 400 or higher + } catch (IOException \| IllegalArgumentException e) { + // Using SLF4J logger for logging the exception + logger.warn("Error checking link: " + linkURL, e); // Log the exception with warning level + return true; // Return true if any exception occurs (link is considered broken) Using `System.err.println` for logging is not consistent with Selenium's logging approach. Consider using a proper logging framework or mechanism instead. java/src/org/openqa/selenium/support/BrokenLinkChecker.java [36-39] -catch (IOException e) { - System.err.println("Error checking link: " + linkURL + " → " + e.getMessage()); +catch (IOException \| IllegalArgumentException e) { + // Consider using a proper logging mechanism instead of System.err + // For example: logger.warn("Error checking link: " + linkURL, e); return true; } `[To ensure code accuracy, apply this suggestion manually]` Suggestion importance[1-10]: 5 __ Why: The suggestion correctly identifies that using System.err.println is not consistent with standard logging practices in libraries like Selenium. However, it only suggests a comment rather than providing a concrete implementation of a proper logging solution.	Low
Update

OmkarGharat

Applied review changes

cgoldberg · 2025-04-01T17:56:53Z

I certainly don't speak for the project, but I would be inclined to reject this PR because it doesn't align with Selenium's goals and is also only implemented in one language binding.

This looks like a useful feature, but it belongs in your test framework, not Selenium itself.

titusfortner · 2025-04-02T20:45:40Z

Thank you, @OmkarGharat for this code suggestion.

The support packages contain example code that many users find helpful, but they do not necessarily represent
the best practices for using Selenium, and the Selenium team is not currently merging changes to them.

We actively encourage people to add the wrapper and helper code that makes sense for them to their own frameworks.
If you have any questions, please contact us

Added BrokenLinkChecker utility to verify broken links

a330353

qodo-merge-pro bot added the Review effort 2/5 label Apr 1, 2025

Update BrokenLinkChecker.java

5e85c3f

OmkarGharat commented Apr 1, 2025

View reviewed changes

Merge branch 'trunk' into trunk

2a82254

titusfortner added B-support Issue or PR related to support classes and removed B-support Issue or PR related to support classes labels Apr 2, 2025

Merge branch 'trunk' into trunk

74cbed9

titusfortner added B-support Issue or PR related to support classes and removed B-support Issue or PR related to support classes labels Apr 2, 2025

Merge branch 'trunk' into trunk

be2ffa7

titusfortner added B-support Issue or PR related to support classes and removed B-support Issue or PR related to support classes labels Apr 2, 2025

Merge branch 'trunk' into trunk

7073305

titusfortner added B-support Issue or PR related to support classes and removed B-support Issue or PR related to support classes labels Apr 2, 2025

Merge branch 'trunk' into trunk

0ad66e7

titusfortner added the B-support Issue or PR related to support classes label Apr 2, 2025

titusfortner closed this Apr 2, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Added BrokenLinkChecker utility to verify broken links #15543

Added BrokenLinkChecker utility to verify broken links #15543

Uh oh!

OmkarGharat commented Apr 1, 2025 •

edited by qodo-merge-pro bot

Loading

Uh oh!

qodo-merge-pro bot commented Apr 1, 2025

Uh oh!

qodo-merge-pro bot commented Apr 1, 2025 •

edited

Loading

Uh oh!

OmkarGharat left a comment

Uh oh!

cgoldberg commented Apr 1, 2025

Uh oh!

titusfortner commented Apr 2, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Added BrokenLinkChecker utility to verify broken links #15543

Added BrokenLinkChecker utility to verify broken links #15543

Uh oh!

Conversation

OmkarGharat commented Apr 1, 2025 • edited by qodo-merge-pro bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

User description

Contributing to Selenium!

Motivation and Context

Types of changes

Checklist

PR Type

Description

Changes walkthrough 📝

Uh oh!

qodo-merge-pro bot commented Apr 1, 2025

PR Reviewer Guide 🔍

Uh oh!

qodo-merge-pro bot commented Apr 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Code Suggestions ✨

Uh oh!

OmkarGharat left a comment

Choose a reason for hiding this comment

Uh oh!

cgoldberg commented Apr 1, 2025

Uh oh!

titusfortner commented Apr 2, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

OmkarGharat commented Apr 1, 2025 •

edited by qodo-merge-pro bot

Loading

qodo-merge-pro bot commented Apr 1, 2025 •

edited

Loading