Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
[JENKINS-48725] fix listener exceptions not being handled properly
  • Loading branch information
Greybird committed Dec 30, 2017
1 parent f2cfda7 commit fe40d4f
Show file tree
Hide file tree
Showing 2 changed files with 140 additions and 5 deletions.
14 changes: 11 additions & 3 deletions src/main/java/org/jvnet/hudson/reactor/Reactor.java
Expand Up @@ -173,7 +173,11 @@ private synchronized Node milestone(final Milestone m) {
if (n==null) {
milestones.put(m,n=new Node(new Runnable() {
public void run() {
listener.onAttained(m);
try {
listener.onAttained(m);
} catch(Throwable x) {
throw new TunnelException(x);
}
}
public String toString() {
return "Milestone:"+m.toString();
Expand Down Expand Up @@ -205,13 +209,17 @@ public synchronized void addAll(Iterable<? extends Task> _tasks) {
for (final Task t : _tasks) {
Node n = new Node(new Runnable() {
public void run() {
listener.onTaskStarted(t);
try {
listener.onTaskStarted(t);
runTask(t);
listener.onTaskCompleted(t);
} catch (Throwable x) {
boolean fatal = t.failureIsFatal();
listener.onTaskFailed(t,x, fatal);
try {
listener.onTaskFailed(t, x, fatal);
} catch(Throwable x2) {
x = x2;
}
if (fatal)
throw new TunnelException(x);
}
Expand Down
131 changes: 129 additions & 2 deletions src/test/java/org/jvnet/hudson/reactor/SessionTest.java
Expand Up @@ -24,10 +24,12 @@
package org.jvnet.hudson.reactor;

import junit.framework.TestCase;
import org.objectweb.carol.cmi.Naming;
import org.objectweb.carol.cmi.test.TeeWriter;
import org.jvnet.hudson.reactor.TaskGraphBuilder.Handle;

import javax.naming.NamingException;
import java.io.IOException;
import java.io.OutputStreamWriter;
import java.io.PrintWriter;
import java.io.StringWriter;
Expand Down Expand Up @@ -59,11 +61,15 @@ public void run(Reactor session, String id) throws Exception {
}

private String execute(Reactor s) throws Exception {
return execute(s, null);
}

private String execute(Reactor s, ReactorListener l) throws Exception {
StringWriter sw = new StringWriter();
System.out.println("----");
final PrintWriter w = new PrintWriter(new TeeWriter(sw,new OutputStreamWriter(System.out)),true);

s.execute(Executors.newCachedThreadPool(),new ReactorListener() {
ReactorListener listener = new ReactorListener() {

//TODO: Does it really needs handlers to be synchronized?
@Override
Expand All @@ -85,7 +91,11 @@ public synchronized void onTaskFailed(Task t, Throwable err, boolean fatal) {
public synchronized void onAttained(Milestone milestone) {
w.println("Attained "+milestone);
}
});
};
if (l != null) {
listener = new ReactorListener.Aggregator(Arrays.asList(listener, l));
}
s.execute(Executors.newCachedThreadPool(), listener);
return sw.toString();
}

Expand Down Expand Up @@ -126,6 +136,89 @@ public void run(Reactor reactor, String id) throws Exception {
}
}

/**
* Is the exception in listener's onTaskStarted properly forwarded?
*/
public void testListenerOnTaskStartedFailure() throws Exception {
final Error[] e = new Error[1];
try {
execute(buildSession("->t1->m", createNoOp()), new ReactorListenerBase() {
@Override
public void onTaskStarted(Task t) {
throw e[0]=new AssertionError("Listener error");
}
}

);
fail();
} catch (ReactorException x) {
assertSame(e[0],x.getCause());
}
}

/**
* Is the exception in listener's onTaskCompleted properly forwarded?
*/
public void testListenerOnTaskCompletedFailure() throws Exception {
final Error[] e = new Error[1];
try {
execute(buildSession("->t1->m", createNoOp()), new ReactorListenerBase() {
@Override
public void onTaskCompleted(Task t) {
throw e[0]=new AssertionError("Listener error");
}
}

);
fail();
} catch (ReactorException x) {
assertSame(e[0],x.getCause());
}
}

/**
* Is the exception in listener's onTaskFailed properly forwarded?
*/
public void testListenerOnTaskFailedFailure() throws Exception {
final Error[] e = new Error[1];
try {
execute(buildSession("->t1->m", new TestTask() {
public void run(Reactor reactor, String id) throws Exception {
throw new IOException("Yep");
}
}), new ReactorListenerBase() {
@Override
public void onTaskFailed(Task t, Throwable err, boolean fatal) {
throw e[0]=new AssertionError("Listener error");
}
}

);
fail();
} catch (ReactorException x) {
assertSame(e[0],x.getCause());
}
}

/**
* Is the exception in listener's onAttained properly forwarded?
*/
public void testListenerOnAttainedFailure() throws Exception {
final Error[] e = new Error[1];
try {
execute(buildSession("->t1->m", createNoOp()), new ReactorListenerBase() {
@Override
public void onAttained(Milestone milestone) {
throw e[0]=new AssertionError("Listener error");
}
}
);
fail();
} catch (ReactorException x) {
assertSame(e[0],x.getCause());
}
}

/**
* Dynamically add a new task that can run immediately.
*/
Expand Down Expand Up @@ -237,6 +330,18 @@ public void run(Reactor reactor, String id) throws InterruptedException {
};
}

/**
* Creates {@link TestTask} that does nothing.
*/
private TestTask createNoOp() {
return new TestTask() {
@Override
public void run(Reactor reactor, String id) throws Exception {
// do nothing
}
};
}

interface TestTask {
void run(Reactor reactor, String id) throws Exception;
}
Expand Down Expand Up @@ -330,4 +435,26 @@ private static String normalizeLineEnds(String s) {
}
return s.replace("\r\n", "\n").replace('\r', '\n');
}

private abstract class ReactorListenerBase implements ReactorListener {
@Override
public void onTaskStarted(Task t) {

}

@Override
public void onTaskCompleted(Task t) {

}

@Override
public void onTaskFailed(Task t, Throwable err, boolean fatal) {

}

@Override
public void onAttained(Milestone milestone) {

}
}
}

0 comments on commit fe40d4f

Please sign in to comment.