Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove redundant error propagation check. #9143

Closed
wants to merge 1 commit into from

Conversation

nk1506
Copy link
Contributor

@nk1506 nk1506 commented Nov 24, 2023

Throwables.propagate always translate to RuntimeException
Adding another check as Throwables.propagateIfInstanceOf(e.getCause(), RuntimeException.class); is redundant.

@@ -58,7 +58,6 @@ public C newInstanceChecked(Object... args) throws Exception {
throw e;
} catch (InvocationTargetException e) {
Throwables.propagateIfInstanceOf(e.getCause(), Exception.class);
Throwables.propagateIfInstanceOf(e.getCause(), RuntimeException.class);
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this line is never reached as the previous line always catches even RuntimeExceptions, so it is safe to remove this.

@@ -62,7 +62,6 @@ public <R> R invokeChecked(Object target, Object... args) throws Exception {

} catch (InvocationTargetException e) {
Throwables.propagateIfInstanceOf(e.getCause(), Exception.class);
Throwables.propagateIfInstanceOf(e.getCause(), RuntimeException.class);
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it is ok to remove this - see comment above

Copy link

github-actions bot commented Oct 6, 2024

This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@iceberg.apache.org list. Thank you for your contributions.

@github-actions github-actions bot added the stale label Oct 6, 2024
Copy link

This pull request has been closed due to lack of activity. This is not a judgement on the merit of the PR in any way. It is just a way of keeping the PR queue manageable. If you think that is incorrect, or the pull request requires review, you can revive the PR at any time.

@github-actions github-actions bot closed this Oct 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants