Changeset 1885 in josm

Timestamp:

2009-08-02T20:15:38+02:00 (15 years ago)

Author:

Gubaer

Message:

Improved exception handling

Location:

trunk/src/org/openstreetmap/josm

Files:

• actions/ApiPreconditionChecker.java (modified) (2 diffs)
• actions/UploadAction.java (modified) (13 diffs)
• actions/downloadtasks/DownloadOsmTask.java (modified) (6 diffs)
• actions/downloadtasks/DownloadOsmTaskList.java (modified) (2 diffs)
• gui/ExceptionDialogUtil.java (modified) (6 diffs)

trunk/src/org/openstreetmap/josm/actions/ApiPreconditionChecker.java

@@ -15 +15 @@
 import org.openstreetmap.josm.data.osm.OsmPrimitive;
 import org.openstreetmap.josm.data.osm.Way;
+import org.openstreetmap.josm.gui.ExceptionDialogUtil;
 import org.openstreetmap.josm.gui.OptionPaneUtil;
 import org.openstreetmap.josm.io.OsmApi;
@@ -61 +62 @@
             }
         } catch (OsmApiInitializationException e) {
-            e.printStackTrace();
-            OptionPaneUtil.showMessageDialog(
-                    Main.parent,
-                    tr("Failed to initialize API. Please try again later."),
-                    tr("API initialization failed"),
-                    JOptionPane.ERROR_MESSAGE
-            );
+            ExceptionDialogUtil.explainOsmTransferException(e);
             return false;
         }

trunk/src/org/openstreetmap/josm/actions/UploadAction.java

@@ -23 +23 @@
 import javax.swing.JOptionPane;
 import javax.swing.JPanel;
-import javax.swing.JRadioButton;
 import javax.swing.JScrollPane;
 
@@ -29 +28 @@
 import org.openstreetmap.josm.data.conflict.ConflictCollection;
 import org.openstreetmap.josm.data.osm.OsmPrimitive;
+import org.openstreetmap.josm.gui.ExceptionDialogUtil;
 import org.openstreetmap.josm.gui.ExtendedDialog;
 import org.openstreetmap.josm.gui.OptionPaneUtil;
@@ -166 +166 @@
                 return;
 
-        final OsmServerWriter server = new OsmServerWriter();
         final Collection<OsmPrimitive> all = new LinkedList<OsmPrimitive>();
         all.addAll(add);
@@ -172 +171 @@
         all.addAll(delete);
 
-        class UploadDiffTask extends  PleaseWaitRunnable {
-
-            private boolean uploadCancelled = false;
-            private boolean uploadFailed = false;
-            private Exception lastException = null;
-
-            public UploadDiffTask() {
-                super(tr("Uploading"),false /* don't ignore exceptions */);
-            }
-
-            @Override protected void realRun() throws SAXException, IOException {
-                try {
-                    server.uploadOsm(getCurrentDataSet().version, all, progressMonitor.createSubTaskMonitor(ProgressMonitor.ALL_TICKS, false));
-                    getEditLayer().cleanData(server.processed, !add.isEmpty());
-                } catch (Exception sxe) {
-                    if (uploadCancelled) {
-                        System.out.println("Ignoring exception caught because upload is cancelled. Exception is: " + sxe.toString());
-                        return;
-                    }
-                    uploadFailed = true;
-                    lastException = sxe;
-                }
-            }
-
-            @Override protected void finish() {
-                if (uploadFailed) {
-                    handleFailedUpload(lastException);
-                }
-            }
-
-            @Override protected void cancel() {
-                server.disconnectActiveConnection();
-                uploadCancelled = true;
-            }
-        }
-
-        Main.worker.execute(new UploadDiffTask());
+        Main.worker.execute(new UploadDiffTask(all));
     }
 
@@ -272 +235 @@
         );
         switch(ret) {
-            case JOptionPane.CLOSED_OPTION: return;
-            case JOptionPane.CANCEL_OPTION: return;
-            case 0: synchronizePrimitive(id); break;
-            case 1: synchronizeDataSet(); break;
-            default:
-                // should not happen
-                throw new IllegalStateException(tr("unexpected return value. Got {0}", ret));
+        case JOptionPane.CLOSED_OPTION: return;
+        case JOptionPane.CANCEL_OPTION: return;
+        case 0: synchronizePrimitive(id); break;
+        case 1: synchronizeDataSet(); break;
+        default:
+            // should not happen
+            throw new IllegalStateException(tr("unexpected return value. Got {0}", ret));
         }
     }
@@ -311 +274 @@
         );
         switch(ret) {
-            case JOptionPane.CLOSED_OPTION: return;
-            case 1: return;
-            case 0: synchronizeDataSet(); break;
-            default:
-                // should not happen
-                throw new IllegalStateException(tr("unexpected return value. Got {0}", ret));
+        case JOptionPane.CLOSED_OPTION: return;
+        case 1: return;
+        case 0: synchronizeDataSet(); break;
+        default:
+            // should not happen
+            throw new IllegalStateException(tr("unexpected return value. Got {0}", ret));
         }
     }
@@ -338 +301 @@
 
     /**
-     * Handles an upload error due to a violated precondition, i.e. a HTTP return code 412
-     *
-     * @param e the exception
-     */
-    protected void handlePreconditionFailed(OsmApiException e) {
-        OptionPaneUtil.showMessageDialog(
-                Main.parent,
-                tr("<html>Uploading to the server <strong>failed</strong> because your current<br>"
-                        +"dataset violates a precondition.<br>"
-                        +"The error message is:<br>"
-                        + "{0}"
-                        + "</html>",
-                        e.getMessage().replace("&", "&amp;").replace("<", "&lt;").replace(">", "&gt;")
-                ),
-                tr("Precondition violation"),
-                JOptionPane.ERROR_MESSAGE
-        );
-        e.printStackTrace();
-    }
-
-
-    /**
      * Handles an error due to a delete request on an already deleted
      * {@see OsmPrimitive}, i.e. a HTTP response code 410, where we know what
@@ -375 +316 @@
         UpdateSelectionAction act = new UpdateSelectionAction();
         act.handlePrimitiveGoneException(Long.parseLong(id));
-    }
-
-    /**
-     * handles the case of an error due to a delete request on an already deleted
-     * {@see OsmPrimitive}, i.e. a HTTP response code 410, where we don't know which
-     * {@see OsmPrimitive} is causing the error.
-     *
-     * @param e the exception
-     */
-    protected void handleGoneForUnknownPrimitive(OsmApiException e) {
-        String msg =  tr("<html>Uploading <strong>failed</strong> because a primitive you tried to<br>"
-                + "delete on the server is already deleted.<br>"
-                + "<br>"
-                + "The error message is:<br>"
-                + "{0}"
-                + "</html>",
-                e.getMessage().replace("&", "&amp;").replace("<", "&lt;").replace(">", "&gt;")
-        );
-        OptionPaneUtil.showMessageDialog(
-                Main.parent,
-                msg,
-                tr("Primitive already deleted"),
-                JOptionPane.ERROR_MESSAGE
-        );
-
     }
 
@@ -418 +334 @@
         } else {
             logger.warning(tr("Error header \"{0}\" does not match expected pattern \"{1}\"",e.getErrorHeader(), pattern));
-            handleGoneForUnknownPrimitive(e);
+            ExceptionDialogUtil.explainGoneForUnknownPrimitive(e);
         }
     }
@@ -432 +348 @@
         //
         if (e instanceof OsmApiInitializationException) {
-            handleOsmApiInitializationException((OsmApiInitializationException)e);
+            ExceptionDialogUtil.explainOsmApiInitializationException((OsmApiInitializationException)e);
             return;
         }
@@ -448 +364 @@
             //
             else if (ex.getResponseCode() == HttpURLConnection.HTTP_PRECON_FAILED) {
-                handlePreconditionFailed(ex);
+                ExceptionDialogUtil.explainPreconditionFailed(ex);
                 return;
             }
@@ -478 +394 @@
         }
 
-        // For any other exception just notify the user
-        //
-        String msg = e.getMessage();
-        if (msg == null) {
-            msg = e.toString();
-        }
-        e.printStackTrace();
-        OptionPaneUtil.showMessageDialog(
-                Main.map,
-                msg,
-                tr("Upload to OSM API failed"),
-                JOptionPane.ERROR_MESSAGE
-        );
-    }
-
-    /**
-     * handles an exception caught during OSM API initialization
-     *
-     * @param e the exception
-     */
-    protected void handleOsmApiInitializationException(OsmApiInitializationException e) {
-        OptionPaneUtil.showMessageDialog(
-                Main.parent,
-                tr(   "Failed to initialize communication with the OSM server {0}.\n"
-                        + "Check the server URL in your preferences and your internet connection.",
-                        Main.pref.get("osm-server.url", "http://api.openstreetmap.org/api")
-                ),
-                tr("Error"),
-                JOptionPane.ERROR_MESSAGE
-        );
-        e.printStackTrace();
+        ExceptionDialogUtil.explainException(e);
     }
 
@@ -634 +520 @@
         }
     }
+
+
+    class UploadDiffTask extends  PleaseWaitRunnable {
+        private boolean uploadCancelled = false;
+        private Exception lastException = null;
+        private Collection <OsmPrimitive> toUpload;
+        private OsmServerWriter writer;
+
+        public UploadDiffTask(Collection <OsmPrimitive> toUpload) {
+            super(tr("Uploading"),false /* don't ignore exceptions */);
+            this.toUpload = toUpload;
+        }
+
+        @Override protected void realRun() throws SAXException, IOException {
+            writer = new OsmServerWriter();
+            try {
+                writer.uploadOsm(getCurrentDataSet().version, toUpload, progressMonitor.createSubTaskMonitor(ProgressMonitor.ALL_TICKS, false));
+                getEditLayer().cleanData(writer.processed, !toUpload.isEmpty());
+            } catch (Exception sxe) {
+                if (uploadCancelled) {
+                    System.out.println("Ignoring exception caught because upload is cancelled. Exception is: " + sxe.toString());
+                    return;
+                }
+                lastException = sxe;
+            }
+        }
+
+        @Override protected void finish() {
+            if (uploadCancelled)
+                return;
+            if (lastException != null) {
+                handleFailedUpload(lastException);
+            }
+        }
+
+        @Override protected void cancel() {
+            uploadCancelled = true;
+            if (writer != null) {
+                writer.disconnectActiveConnection();
+            }
+        }
+    }
 }

trunk/src/org/openstreetmap/josm/actions/downloadtasks/DownloadOsmTask.java

@@ -41 +41 @@
     private Future<Task> task = null;
     private DataSet downloadedData;
+    private boolean canceled = false;
+    private boolean failed = false;
 
     private class Task extends PleaseWaitRunnable {
@@ -46 +48 @@
         private DataSet dataSet;
         private boolean newLayer;
-        private boolean cancelled;
+        private boolean canceled;
         private Exception lastException;
 
@@ -59 +61 @@
                 dataSet = reader.parseOsm(progressMonitor.createSubTaskMonitor(ProgressMonitor.ALL_TICKS, false));
             } catch(Exception e) {
-                if (cancelled) {
+                if (canceled) {
                     logger.warning(tr("Ignoring exception because download has been cancelled. Exception was: {0}" + e.toString()));
                     return;
@@ -102 +104 @@
 
         @Override protected void finish() {
-            if (cancelled)
+            if (canceled)
                 return;
             if (lastException != null) {
                 ExceptionDialogUtil.explainException(lastException);
+                DownloadOsmTask.this.setFailed(true);
                 return;
             }
@@ -135 +138 @@
 
         @Override protected void cancel() {
+            this.canceled = true;
             if (reader != null) {
                 reader.cancel();
             }
+            DownloadOsmTask.this.setCanceled(true);
         }
     }
@@ -144 +149 @@
     private void rememberDownloadedData(DataSet ds) {
         this.downloadedData = ds;
+    }
+
+    public boolean isCanceled() {
+        return canceled;
+    }
+
+    public void setCanceled(boolean canceled) {
+        this.canceled = canceled;
+    }
+
+    public boolean isFailed() {
+        return failed;
+    }
+
+    public void setFailed(boolean failed) {
+        this.failed = failed;
     }
 

trunk/src/org/openstreetmap/josm/actions/downloadtasks/DownloadOsmTaskList.java

@@ -114 +114 @@
         }
 
+        // FIXME: this is a hack. We assume that the user canceled the whole download if at least
+        // one task was canceled or if it failed
+        //
+        for (DownloadTask task: osmTasks) {
+            if (task instanceof DownloadOsmTask) {
+                DownloadOsmTask osmTask = (DownloadOsmTask)task;
+                if (osmTask.isCanceled() || osmTask.isFailed())
+                    return;
+            }
+        }
         Set<Long> myPrimitiveIds = Main.map.mapView.getEditLayer().data.getCompletePrimitiveIds();
         Set<Long> downloadedIds = getDownloadedIds();
@@ -233 +243 @@
             if(task instanceof DownloadOsmTask) {
                 DataSet ds = ((DownloadOsmTask)task).getDownloadedData();
-                ret.addAll(ds.getPrimitiveIds());
+                if (ds != null) {
+                    ret.addAll(ds.getPrimitiveIds());
+                }
             }
         }

trunk/src/org/openstreetmap/josm/gui/ExceptionDialogUtil.java

@@ -4 +4 @@
 import static org.openstreetmap.josm.tools.I18n.tr;
 
+import java.net.HttpURLConnection;
 import java.net.MalformedURLException;
+import java.net.SocketException;
 import java.net.URL;
+import java.net.UnknownHostException;
 
 import javax.swing.JOptionPane;
@@ -11 +14 @@
 import org.openstreetmap.josm.Main;
 import org.openstreetmap.josm.io.OsmApi;
+import org.openstreetmap.josm.io.OsmApiException;
+import org.openstreetmap.josm.io.OsmApiInitializationException;
 import org.openstreetmap.josm.io.OsmTransferException;
 
@@ -23 +28 @@
      */
     private ExceptionDialogUtil() {}
+
+    /**
+     * handles an exception caught during OSM API initialization
+     *
+     * @param e the exception
+     */
+    public static void explainOsmApiInitializationException(OsmApiInitializationException e) {
+        e.printStackTrace();
+        OptionPaneUtil.showMessageDialog(
+                Main.parent,
+                tr(   "Failed to initialize communication with the OSM server {0}.\n"
+                        + "Check the server URL in your preferences and your internet connection.",
+                        Main.pref.get("osm-server.url", "http://api.openstreetmap.org/api")
+                ),
+                tr("Error"),
+                JOptionPane.ERROR_MESSAGE
+        );
+    }
+
+
+    /**
+     * Explains an upload error due to a violated precondition, i.e. a HTTP return code 412
+     *
+     * @param e the exception
+     */
+    public static void explainPreconditionFailed(OsmApiException e) {
+        e.printStackTrace();
+        OptionPaneUtil.showMessageDialog(
+                Main.parent,
+                tr("<html>Uploading to the server <strong>failed</strong> because your current<br>"
+                        +"dataset violates a precondition.<br>"
+                        +"The error message is:<br>"
+                        + "{0}"
+                        + "</html>",
+                        e.getMessage().replace("&", "&amp;").replace("<", "&lt;").replace(">", "&gt;")
+                ),
+                tr("Precondition violation"),
+                JOptionPane.ERROR_MESSAGE
+        );
+    }
 
 
@@ -73 +118 @@
                 JOptionPane.ERROR_MESSAGE
         );
-
+    }
+
+    /**
+     * Explains a {@see SecurityException} which has caused an {@see OsmTransferException}.
+     * This is most likely happening when user tries to access the OSM API from whitin an
+     * applet which wasn't loaded from the API server.
+     * 
+     * @param e the exception
+     */
+
+    public static void explainNestedSocketException(OsmTransferException e) {
+        String apiUrl = OsmApi.getOsmApi().getBaseUrl();
+        String message = tr("<html>Failed to open a connection to the remote server<br>"
+                + "''{0}''.<br>"
+                + "Please check your internet connection.</html>",
+                apiUrl
+        );
+        e.printStackTrace();
+        OptionPaneUtil.showMessageDialog(
+                Main.parent,
+                message,
+                tr("Network exception"),
+                JOptionPane.ERROR_MESSAGE
+        );
+    }
+
+    /**
+     * Explains a {@see SecurityException} which has caused an {@see OsmTransferException}.
+     * This is most likely happening when user tries to access the OSM API from whitin an
+     * applet which wasn't loaded from the API server.
+     * 
+     * @param e the exception
+     */
+
+    public static void explainNestedUnkonwnHostException(OsmTransferException e) {
+        String apiUrl = OsmApi.getOsmApi().getBaseUrl();
+        String host = tr("unknown");
+        try {
+            host = new URL(apiUrl).getHost();
+        } catch(MalformedURLException ex) {
+            // shouldn't happen
+        }
+
+        String message = tr("<html>Failed to open a connection to the remote server<br>"
+                + "''{0}''.<br>"
+                + "Host name ''{1}'' couldn''t be resolved. <br>"
+                + "Please check the API URL in your preferences and your internet connection.</html>",
+                apiUrl, host
+        );
+        e.printStackTrace();
+        OptionPaneUtil.showMessageDialog(
+                Main.parent,
+                message,
+                tr("Unknown host"),
+                JOptionPane.ERROR_MESSAGE
+        );
+    }
+
+    protected static <T> T getNestedException(Exception e, Class<T> nested) {
+        Throwable t = e;
+        while (t != null && !(t.getClass().isAssignableFrom(nested))) {
+            t = t.getCause();
+        }
+        return nested.cast(t);
     }
 
@@ -83 +191 @@
      * @return the first {@see SecurityException} in a chain of nested exceptions
      */
-    protected static SecurityException getSecurityChildException(Exception e) {
-        Throwable t = e;
-        while(t != null && ! (t instanceof SecurityException)) {
-            t = t.getCause();
-        }
-        return (SecurityException)t;
-    }
+    protected static SecurityException getNestedSecurityException(Exception e) {
+        return getNestedException(e, SecurityException.class);
+    }
+
+
+    /**
+     * Replies the first {@see SocketException} in a chain of nested exceptions.
+     * null, if no {@see SocketException} is in this chain.
+     * 
+     * @param e the root exception
+     * @return the first {@see SocketException} in a chain of nested exceptions
+     */
+    protected static SocketException getNestedSocketException(Exception e) {
+        return getNestedException(e, SocketException.class);
+    }
+
+    /**
+     * Replies the first {@see UnknownHostException} in a chain of nested exceptions.
+     * null, if no {@see UnknownHostException} is in this chain.
+     * 
+     * @param e the root exception
+     * @return the first {@see UnknownHostException} in a chain of nested exceptions
+     */
+    protected static UnknownHostException getNestedUnknownHostException(Exception e) {
+        return getNestedException(e, UnknownHostException.class);
+    }
+
 
     /**
@@ -97 +225 @@
      */
     public static void explainOsmTransferException(OsmTransferException e) {
-        if (getSecurityChildException(e) != null) {
+        if (getNestedSecurityException(e) != null) {
             explainSecurityException(e);
             return;
         }
+        if (getNestedSocketException(e) != null) {
+            explainNestedSocketException(e);
+            return;
+        }
+        if (getNestedUnknownHostException(e) != null) {
+            explainNestedUnkonwnHostException(e);
+            return;
+        }
+        if (e instanceof OsmApiInitializationException){
+            explainOsmApiInitializationException((OsmApiInitializationException)e);
+            return;
+        }
+        if (e instanceof OsmApiException) {
+            OsmApiException oae = (OsmApiException)e;
+            if (oae.getResponseCode() == HttpURLConnection.HTTP_PRECON_FAILED) {
+                explainPreconditionFailed(oae);
+                return;
+            }
+            if (oae.getResponseCode() == HttpURLConnection.HTTP_GONE) {
+                explainGoneForUnknownPrimitive(oae);
+                return;
+            }
+        }
+
         explainGeneric(e);
+    }
+
+    /**
+     * explains the case of an error due to a delete request on an already deleted
+     * {@see OsmPrimitive}, i.e. a HTTP response code 410, where we don't know which
+     * {@see OsmPrimitive} is causing the error.
+     *
+     * @param e the exception
+     */
+    public static void explainGoneForUnknownPrimitive(OsmApiException e) {
+        String msg =  tr("<html>Uploading <strong>failed</strong> because a primitive you tried to<br>"
+                + "delete on the server is already deleted.<br>"
+                + "<br>"
+                + "The error message is:<br>"
+                + "{0}"
+                + "</html>",
+                e.getMessage().replace("&", "&amp;").replace("<", "&lt;").replace(">", "&gt;")
+        );
+        OptionPaneUtil.showMessageDialog(
+                Main.parent,
+                msg,
+                tr("Primitive already deleted"),
+                JOptionPane.ERROR_MESSAGE
+        );
+
     }