Skip to content

Commit 8e49f0e

Browse files
committed
Handle exception thrown in ObjectLoader::doInBackground
As it turns out, SwingWorker still called done, even if the body of the task itself threw an exception. So it just pushes the open event, even though `nodes` is null. What's annoying, is that there doesn't seem to be a good mechanism for getting that exception... You either need to handle it yourself within `doInBackground` and then read that exception data in `done`, or abuse `SwingWorker::get` and catch an `ExecutionException`. See issue #36 for discovery.
1 parent 0ac6a61 commit 8e49f0e

File tree

5 files changed

+65
-27
lines changed

5 files changed

+65
-27
lines changed

src/main/java/com/itextpdf/rups/model/IProgressDialog.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -69,11 +69,11 @@ public interface IProgressDialog {
6969
void setTotal(int n);
7070

7171
/**
72-
* Displays an error dialog for the given exception.
72+
* Displays an error dialog for the given throwable.
7373
*
74-
* @param ex exception to display information about
74+
* @param th throwable to display information about
7575
*/
76-
void showErrorDialog(Exception ex);
76+
void showErrorDialog(Throwable th);
7777

7878
/**
7979
* Control the visibility of the progress dialog.

src/main/java/com/itextpdf/rups/model/LoggerHelper.java

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -52,10 +52,10 @@ private LoggerHelper() {
5252
// static class
5353
}
5454

55-
public static void warn(String message, Exception e, String className) {
55+
public static void warn(String message, Throwable th, String className) {
5656
final Logger logger = LoggerFactory.getLogger(className);
5757
logger.warn(message);
58-
logger.debug(message, e);
58+
logger.debug(message, th);
5959
}
6060

6161
public static void warn(String message, String className) {
@@ -64,8 +64,8 @@ public static void warn(String message, String className) {
6464
logger.debug(message);
6565
}
6666

67-
public static void warn(String message, Exception e, Class<?> c) {
68-
warn(message, e, c.getName());
67+
public static void warn(String message, Throwable th, Class<?> c) {
68+
warn(message, th, c.getName());
6969
}
7070

7171
public static void warn(String message, Class<?> c) {
@@ -80,10 +80,10 @@ public static void warnf(Language format, Class<?> c, Object... args) {
8080
warnf(format.getString(), c, args);
8181
}
8282

83-
public static void error(String message, Exception e, String className) {
83+
public static void error(String message, Throwable th, String className) {
8484
final Logger logger = LoggerFactory.getLogger(className);
8585
logger.error(message);
86-
logger.debug(message, e);
86+
logger.debug(message, th);
8787
}
8888

8989
public static void error(String message, String className) {
@@ -92,8 +92,8 @@ public static void error(String message, String className) {
9292
logger.debug(message);
9393
}
9494

95-
public static void error(String message, Exception e, Class<?> c) {
96-
error(message, e, c.getName());
95+
public static void error(String message, Throwable th, Class<?> c) {
96+
error(message, th, c.getName());
9797
}
9898

9999
public static void error(String message, Class<?> c) {
@@ -110,12 +110,12 @@ public static void info(String message, Class<?> c) {
110110
info(message, c.getName());
111111
}
112112

113-
public static void debug(String message, Exception e, String className) {
114-
LoggerFactory.getLogger(className).debug(message, e);
113+
public static void debug(String message, Throwable th, String className) {
114+
LoggerFactory.getLogger(className).debug(message, th);
115115
}
116116

117-
public static void debug(String message, Exception e, Class<?> c) {
118-
debug(message, e, c.getName());
117+
public static void debug(String message, Throwable th, Class<?> c) {
118+
debug(message, th, c.getName());
119119
}
120120

121121
public static void debug(String message, String className) {

src/main/java/com/itextpdf/rups/model/ObjectLoader.java

Lines changed: 45 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -44,29 +44,32 @@ This file is part of the iText (R) project.
4444

4545
import com.itextpdf.rups.view.Language;
4646

47+
import java.util.concurrent.ExecutionException;
48+
import java.util.concurrent.TimeUnit;
49+
import java.util.concurrent.TimeoutException;
4750
import javax.swing.SwingUtilities;
4851
import javax.swing.SwingWorker;
4952

5053
/**
5154
* Loads the necessary iText PDF objects in Background.
5255
*/
53-
public class ObjectLoader extends SwingWorker<Void, Void> {
56+
public final class ObjectLoader extends SwingWorker<Void, Void> {
5457
/**
5558
* This is the object that wait for task to complete.
5659
*/
57-
protected IRupsEventListener eventListener;
60+
private final IRupsEventListener eventListener;
5861
/**
5962
* RUPS's PdfFile object.
6063
*/
61-
protected IPdfFile file;
64+
private final IPdfFile file;
6265
/**
6366
* The factory that can provide PDF objects.
6467
*/
65-
protected IndirectObjectFactory objects;
68+
private IndirectObjectFactory objects;
6669
/**
6770
* The factory that can provide tree nodes.
6871
*/
69-
protected TreeNodeFactory nodes;
72+
private TreeNodeFactory nodes;
7073
/**
7174
* a human readable name for this loaded
7275
*/
@@ -145,12 +148,47 @@ protected Void doInBackground() {
145148

146149
@Override
147150
protected void done() {
151+
if (handleTaskException()) {
152+
return;
153+
}
148154
try {
149155
eventListener.handleOpenDocument(this);
156+
progress.setVisible(false);
150157
} catch (RuntimeException ex) {
151-
progress.showErrorDialog(ex);
152-
LoggerHelper.error(ex.getLocalizedMessage(), ex, getClass());
158+
displayThrowable(ex);
159+
}
160+
}
161+
162+
private boolean handleTaskException() {
163+
try {
164+
// This will throw an ExecutionException, if doInBackground
165+
// threw an exception
166+
get(0L, TimeUnit.NANOSECONDS);
167+
return false;
168+
} catch (ExecutionException e) {
169+
displayThrowable(e.getCause());
170+
clearResult();
171+
} catch (TimeoutException | RuntimeException e) {
172+
// This should not happen in practice
173+
displayThrowable(e);
174+
clearResult();
175+
} catch (InterruptedException e) {
176+
// This should not happen in practice
177+
displayThrowable(e);
178+
clearResult();
179+
Thread.currentThread().interrupt();
153180
}
181+
return true;
182+
}
183+
184+
private void clearResult() {
185+
objects = null;
186+
nodes = null;
187+
}
188+
189+
private void displayThrowable(Throwable th) {
190+
LoggerHelper.error(th.getLocalizedMessage(), th, getClass());
191+
progress.showErrorDialog(th);
154192
progress.setVisible(false);
155193
}
156194
}

src/main/java/com/itextpdf/rups/model/ProgressDialog.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -158,12 +158,12 @@ public void setTotal(int n) {
158158
}
159159

160160
/**
161-
* Displays an error dialog for the given exception.
161+
* Displays an error dialog for the given throwable.
162162
*
163-
* @param ex exception to display information about
163+
* @param th throwable to display information about
164164
*/
165165
@Override
166-
public void showErrorDialog(Exception ex) {
167-
ErrorDialogPane.showErrorDialog(this, ex);
166+
public void showErrorDialog(Throwable th) {
167+
ErrorDialogPane.showErrorDialog(this, th);
168168
}
169169
}

src/test/java/com/itextpdf/rups/mock/NoopProgressDialog.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ public void setTotal(int n) {
6464
}
6565

6666
@Override
67-
public void showErrorDialog(Exception ex) {
67+
public void showErrorDialog(Throwable th) {
6868
// noop
6969
}
7070

0 commit comments

Comments
 (0)