-
-
Notifications
You must be signed in to change notification settings - Fork 8.6k
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
Capturing runtime exceptions from Computer.threadPoolForRemoting.submit(Runnable)
#7284
Capturing runtime exceptions from Computer.threadPoolForRemoting.submit(Runnable)
#7284
Conversation
core/src/main/java/hudson/util/ExceptionCatchingThreadFactory.java
Outdated
Show resolved
Hide resolved
Co-authored-by: Alexander Brandes <brandes.alexander@web.de>
@@ -131,9 +131,12 @@ protected void opened() { | |||
LOGGER.fine(() -> "setting up channel for " + agent); | |||
state.fireBeforeChannel(new ChannelBuilder(agent, Computer.threadPoolForRemoting)); | |||
transport = new Transport(); | |||
state.fireAfterChannel(state.getChannelBuilder().build(transport)); | |||
LOGGER.fine(() -> "set up channel for " + agent); | |||
return null; |
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.
This was coercing the lambda to a Callable
based on the checked exception.
core/src/main/java/hudson/util/ExceptionCatchingThreadFactory.java
Outdated
Show resolved
Hide resolved
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.
/label ready-for-merge
This PR is now ready for merge, after ~24 hours, we will merge it if there's no negative feedback.
Thanks!
@Vlatombe do you mind taking a look, please? I was holding off a merge, because Jesse requested your explicit review. |
Spent a while trying to track down an issue which turned out to be an unchecked exception thrown out of
JnlpConnectionStateListener.afterChannel
and swallowed byFutureTask
, whichExceptionCatchingThreadFactory
does not address.Proposed changelog entries
Computer.threadPoolForRemoting
.Maintainer checklist
Before the changes are marked as
ready-for-merge
:upgrade-guide-needed
label is set and there is a Proposed upgrade guidelines section in the pull request title (see example).lts-candidate
to be considered (see query).