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

Fix rec catch exception #118

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

iqbalrohail
Copy link
Contributor

done the code fix to overcome the spot bug error for REC_CATCH_EXCEPTION

This exception is thrown when we use catch(Exception e) to catch all types of exceptions at once, it could mask actual programming problems, so spotbugs ask you to catch the specific exception so that run-time exceptions can be thrown which indicate programming problems.

  • Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
  • Ensure that the pull request title represents the desired changelog entry
  • Please describe what you did

@iqbalrohail iqbalrohail requested a review from a team as a code owner October 29, 2022 14:24
Comment on lines +38 to +41
}catch(RuntimeException e){
throw new RuntimeException(e);
}
catch (Exception e) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't see the benefit of this change in reducing the likelihood of issues. Can you explain further or point me to a location where I can read more about the benefit of this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As spotbug spots this issue of REC_CATCH_EXCEPTION so rather than ignoring that bug in excludes file i think it's a good approach to make a code change to solve that issue

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Adding this will catch the runtime exceptions and make the issue solve for reference you can check Alluxio/alluxio#10459

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@MarkEWaite doesn't it seems good to have this change?

@MarkEWaite MarkEWaite added the chore Maintenance label Nov 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore Maintenance
Projects
None yet
2 participants