-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
core: Catch uncaught exceptions in ServerImpl #6165
Conversation
Ping @ejona86 |
The discussion for whether we are going to do this is still ongoing in #6162 |
So I took another look at this. I'm really surprised that Java 8 with source=7 has the type inferencing to notice that "no checked exceptions can be thrown" within the block, so even though we Since all the tests pass, this looks like a general cleanup, and so would be good to merge. #6162 is still about whether we'll go out of our way to handle checked-but-not-checked exceptions, but any place Java isn't a PITA when re-throwing and we can have one code path, that would be good. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approving from a code-cleanup standpoint.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Fixes #6162A cleanup regarding #6162