Graceful exception handling in Swing Worker

Question

I am using threading in application through Swing Worker class. It works fine, yet I have a bad feeling about showing an error message dialog in try-catch block. Can it potentially block the application? This is what it looks right now:

SwingWorker<Void, Void> worker = new SwingWorker<Void, Void>() {      // Executed in background thread     public Void doInBackground() {         try {             DoFancyStuff();         } catch (Exception e) {              e.printStackTrace();              String msg = String.format("Unexpected problem: %s", e                     .toString());              //TODO: executed in background thread and should be executed in EDT?             JOptionPane.showMessageDialog(Utils.getActiveFrame(),                     msg, "Error", JOptionPane.ERROR_MESSAGE,                     errorIcon);          }//END: try-catch          return null;     }      // Executed in event dispatch thread     public void done() {         System.out.println("Done");     } }; 

Can it be done in a safe way using Swing Worker framework? Is overriding publish() method a good lead here?

EDIT:

Did it like this:

} catch (final Exception e) {      SwingUtilities.invokeLater(new Runnable() {          public void run() {              e.printStackTrace();              String msg = String.format(                     "Unexpected problem: %s", e.toString());              JOptionPane.showMessageDialog(Utils                     .getActiveFrame(), msg, "Error",                     JOptionPane.ERROR_MESSAGE, errorIcon);          }     });  } 

Calling get in done method would result in two try-catch blocks, as the computational part throws exceptions, so I think this is cleaner in the end.

Answer 1

The right way to do it is as follows:

SwingWorker<Void, Void> worker = new SwingWorker<Void, Void>() {     // Executed in background thread     protected Void doInBackground() throws Exception {         DoFancyStuff();         return null;     }      // Executed in EDT     protected void done() {         try {             System.out.println("Done");             get();         } catch (ExecutionException e) {             e.getCause().printStackTrace();             String msg = String.format("Unexpected problem: %s",                             e.getCause().toString());             JOptionPane.showMessageDialog(Utils.getActiveFrame(),                 msg, "Error", JOptionPane.ERROR_MESSAGE, errorIcon);         } catch (InterruptedException e) {             // Process e here         }     } } 

You should NOT try to catch exceptions in the background thread but rather let them pass through to the SwingWorker itself, and then you can get them in the done() method by calling get()which normally returns the result of doInBackground() (Voidin your situation). If an exceptionwas thrown in the background thread then get() will throw it, wrapped inside an ExecutionException.

Please also note that overidden SwingWorker methods are protected and you don't need to make them public.