Skip to content

[refactoring] improve handling exceptions#10068

Merged
diemol merged 2 commits intoSeleniumHQ:trunkfrom
asolntsev:improve-exception-handling
Dec 8, 2021
Merged

[refactoring] improve handling exceptions#10068
diemol merged 2 commits intoSeleniumHQ:trunkfrom
asolntsev:improve-exception-handling

Conversation

@asolntsev
Copy link
Copy Markdown
Contributor

@asolntsev asolntsev commented Nov 20, 2021

  • throw "new UncheckedIOException(ioe)"" instead of just "new RuntimeException(ioe)"
  • avoid catching too broad Exception which causes unneeded re-wrapping of RuntimeExceptions

Description

it's just a small refactoring: improving some of "catch Exception" blocks.

P.S. Replacing RuntimeException by UncheckedIOException doesn't explicitly solve any practical problem, it's just a better exception for wrapping IOException.

You Many coding standards recommend to avoid throwing too generic exceptions like RuntimeException or Exception. And UncheckedIOException is exactly for that: it's a "RuntimeException for wrapping IOException".

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.

Comment thread java/test/org/openqa/selenium/remote/tracing/opentelemetry/TracerTest.java Outdated
Comment thread java/src/com/thoughtworks/selenium/webdriven/JavascriptLibrary.java Outdated
private String pathToServlet;
private String browserStartCommand;
private String browserURL;
private final String browserStartCommand;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please revert changes to files in the thoughtworks package? This is legacy code and we will eventually phase out RC.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, I will revert.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@diemol Done (reverted changes in com.thoughtworks package).

Comment thread java/src/com/thoughtworks/selenium/webdriven/JavascriptLibrary.java Outdated
Comment thread java/src/org/openqa/selenium/net/UrlChecker.java
Comment thread java/test/com/thoughtworks/selenium/CSVTest.java Outdated
* throw "new UncheckedIOException(ioe)"" instead of just "new RuntimeException(ioe)"
* avoid catching too broad Exception which causes unneeded re-wrapping of RuntimeExceptions
.. as it's deprecated and will be removed anyway.
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented Dec 3, 2021

SonarCloud Quality Gate failed.    Quality Gate failed

Bug C 1 Bug
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

@diemol diemol merged commit 04a681b into SeleniumHQ:trunk Dec 8, 2021
@asolntsev asolntsev deleted the improve-exception-handling branch December 8, 2021 11:17
elgatov pushed a commit to elgatov/selenium that referenced this pull request Jun 27, 2022
* [refactoring] improve handling exceptions

* throw "new UncheckedIOException(ioe)"" instead of just "new RuntimeException(ioe)"
* avoid catching too broad Exception which causes unneeded re-wrapping of RuntimeExceptions

* revert changes in com.thoughtworks package

.. as it's deprecated and will be removed anyway.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants