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.

Answer 2

One option is to use SwingUtilities.invokeLater(...) to post the action on the EDT

SwingUtilities.invokeLater(new Runnable(){
    @Override
    public void run(){
        JOptionPane.showMessageDialog(
            Utils.getActiveFrame(),
            msg, 
            "Error", 
            JOptionPane.ERROR_MESSAGE,
            errorIcon);
    }
});

And as you noted, SwingWorker is capable of reporting intermediate results, but you'll need to override process(...), which is called when you invoke publish(...).

Regardless, why not just set a flag if an exception occurs, and if that flag is set, show the dialog in done() since it's executed safely in the EDT?

Answer 3

You are right, you are violating the cardinal rule of Swing, which is don't modify the GUI anywhere except for on the event-dispatch-thread.

If it was me, I would throw an event that the GUI listens for to show the error message. Or, you can just wrap the invocation of the SwingWorker in a try catch and show the dialogue there.

Answer 4

First of all: sorry for the short answer, don't have too much time to spare.

I had the same problem: wanting to publish to System.out from within the worker.

Short answer: It won't block your app if you use the execute() method

The thing is that there is no blocking if you execute the worker as it should be: a background task.

class MyWorker extend SwingWorker<Void, Void>{
  @Override
  protected Void doInBackground() throws ... {
    // your logic here and a message to a stream
    System.out.println("from my worker, with love");
    // ...
    try { 
      throw new Exception("Whoops, this is an exception from within the worker"); 
    } catch (Exception e) { 
      System.out.println(e.getMessage()); 
    }
  }
}

Now you will invoke this worker creating a new instance, and after that calling the execute() method. But to save you some time: you will probably want to know when your worker is done, so you'll need to register an property change listener, which is fairly simple:

class MyListener implements PropertyChangeListener{
  @Override
  public void propertyChange(PropertyChangeEvent evt){
    if(evt.getPropertyName().equals("state") && evt.getNewValue().equals(SwingWorker.StateValue.DONE)){
        System.out.println("The worker is done");
    }
  }
}

And to put everything together at your main():

public void main(...){
  MyWorker w = new MyWorker();
  MyListener l = new MyListener();
  w.addPropertyChangeListener(l);
  w.execute();
}