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
[JENKINS-58900] Improve error messages for agent disconnections inside of node #115
Conversation
I filed #116 to hopefully make the build a bit more robust. |
😩 |
@@ -57,7 +59,14 @@ | |||
if (f != null) { | |||
LOGGER.log(Level.FINE, "serving {0}:{1}", new Object[] {r.slave, r.path}); | |||
} else { | |||
LOGGER.log(Level.FINE, "failing to serve {0}:{1}", new Object[] {r.slave, r.path}); |
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.
Not obvious to me if we were returning null
here for any reason in particular. ExecutorStepTest.contextualizeFreshFilePathAfterAgentReconnection
passes with the change, not sure if there are any other scenarios related to FilePathDynamicContext
where the new behavior would be problematic.
src/test/java/org/jenkinsci/plugins/workflow/support/steps/ExecutorStepTest.java
Show resolved
Hide resolved
src/test/java/org/jenkinsci/plugins/workflow/support/steps/ExecutorStepTest.java
Outdated
Show resolved
Hide resolved
Co-Authored-By: Jesse Glick <jglick@cloudbees.com>
if (listener != null) { | ||
OfflineCause oc = c.getOfflineCause(); | ||
if (oc != null) { | ||
listener.getLogger().println(c.getDisplayName() + " was marked offline: " + oc); |
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.
Note that this assumes OfflineCause.toString
has been meaningfully overridden. That is definitely true of SimpleOfflineCause
, and seems to be done for other subtypes as well, though really it ought to have been defined as abstract
to ensure that it is implemented.
See JENKINS-58900.
The problem is that you get a
MissingContextVariableException
where you would expect to get some kind of connection-related exception (seen on some builds of Jenkins core on ci.jenkins.io where the Windows agent apparently disconnected but the error in the logs was aMissingContextVariableException
). I think this is a side effect of #101 (JENKINS-41854), but I haven't investigated enough to know for sure. If so, maybe fixable by throwing some kind of exception instead of just logging here:workflow-durable-task-step-plugin/src/main/java/org/jenkinsci/plugins/workflow/support/steps/FilePathDynamicContext.java
Line 60 in 5840b56
CC @oleg-nenashev