Changeset 6642 in josm for trunk/src

Timestamp:

2014-01-06T15:57:41+01:00 (10 years ago)

Author:

Don-vip

Message:

Improve handling of network errors at startup, suggest to change proxy settings

Location:

trunk/src/org/openstreetmap/josm

Files:

• Main.java (modified) (8 diffs)
• data/imagery/ImageryLayerInfo.java (modified) (1 diff)
• gui/MainApplication.java (modified) (6 diffs)
• io/MirroredInputStream.java (modified) (1 diff)
• io/OsmApi.java (modified) (4 diffs)
• tools/Utils.java (modified) (3 diffs)
• tools/WikiReader.java (modified) (3 diffs)

trunk/src/org/openstreetmap/josm/Main.java

@@ -23 +23 @@
 import java.util.Arrays;
 import java.util.Collection;
+import java.util.HashMap;
 import java.util.Iterator;
 import java.util.List;
@@ -202 +203 @@
      */
     public OsmValidator validator;
+
     /**
      * The MOTD Layer.
@@ -208 +210 @@
 
     private static final Collection<MapFrameListener> mapFrameListeners = new ArrayList<MapFrameListener>();
+
+    protected static final Map<String, Throwable> networkErrors = new HashMap<String, Throwable>();
 
     /**
@@ -303 +307 @@
      */
     public static void error(Throwable t) {
-        error(getErrorMessage(t));
-        t.printStackTrace();
+        error(t, true);
     }
 
@@ -313 +316 @@
      */
     public static void warn(Throwable t) {
+        warn(t, true);
+    }
+
+    /**
+     * Prints an error message for the given Throwable.
+     * @param t The throwable object causing the error
+     * @param stackTrace {@code true}, if the stacktrace should be displayed
+     * @since 6442
+     */
+    public static void error(Throwable t, boolean stackTrace) {
+        error(getErrorMessage(t));
+        if (stackTrace) {
+            t.printStackTrace();
+        }
+    }
+
+    /**
+     * Prints a warning message for the given Throwable.
+     * @param t The throwable object causing the error
+     * @param stackTrace {@code true}, if the stacktrace should be displayed
+     * @since 6442
+     */
+    public static void warn(Throwable t, boolean stackTrace) {
         warn(getErrorMessage(t));
-        t.printStackTrace();
-    }
-
-    private static String getErrorMessage(Throwable t) {
+        if (stackTrace) {
+            t.printStackTrace();
+        }
+    }
+
+    /**
+     * Returns a human-readable message of error, also usable for developers.
+     * @param t The error
+     * @return The human-readable error message
+     * @since 6642
+     */
+    public static String getErrorMessage(Throwable t) {
+        if (t == null) {
+                return null;
+        }
         StringBuilder sb = new StringBuilder(t.getClass().getName());
         String msg = t.getMessage();
@@ -455 +492 @@
                 try {
                     OsmApi.getOsmApi().initialize(null, true);
-                } catch (Exception x) {
-                    // ignore any exception here.
+                } catch (Exception e) {
+                    Main.warn(getErrorMessage(Utils.getRootCause(e)));
                 }
                 return null;
@@ -722 +759 @@
             panel.updateUI();
         } catch (final Exception e) {
-            e.printStackTrace();
+            error(e);
         }
         UIManager.put("OptionPane.okIcon", ImageProvider.get("ok"));
@@ -1350 +1387 @@
         return listener != null ? mapFrameListeners.remove(listener) : false;
     }
+
+    /**
+     * Adds a new network error that occur to give a hint about broken Internet connection.
+     * Do not use this method for errors known for sure thrown because of a bad proxy configuration.
+     * 
+     * @param url The accessed URL that caused the error
+     * @param t The network error
+     * @return The previous error associated to the given resource, if any. Can be {@code null}
+     * @since 6639
+     */
+    public static Throwable addNetworkError(URL url, Throwable t) {
+        if (url != null && t != null) {
+            Throwable old = addNetworkError(url.toExternalForm(), t);
+            if (old != null) {
+                Main.warn("Already here "+old);
+            }
+            return old;
+        }
+        return null;
+    }
+
+    /**
+     * Adds a new network error that occur to give a hint about broken Internet connection.
+     * Do not use this method for errors known for sure thrown because of a bad proxy configuration.
+     * 
+     * @param url The accessed URL that caused the error
+     * @param t The network error
+     * @return The previous error associated to the given resource, if any. Can be {@code null}
+     * @since 6639
+     */
+    public static Throwable addNetworkError(String url, Throwable t) {
+        if (url != null && t != null) {
+            return networkErrors.put(url, t);
+        }
+        return null;
+    }
+
+    /**
+     * Returns the network errors that occured until now.
+     * @return the network errors that occured until now, indexed by URL
+     * @since 6639
+     */
+    public static Map<String, Throwable> getNetworkErrors() {
+        return new HashMap<String, Throwable>(networkErrors);
+    }
 }

trunk/src/org/openstreetmap/josm/data/imagery/ImageryLayerInfo.java

@@ -81 +81 @@
             } catch (IOException ex) {
                 Utils.close(stream);
-                ex.printStackTrace();
+                Main.error(ex, false);
                 continue;
-            } catch (SAXException sex) {
+            } catch (SAXException ex) {
                 Utils.close(stream);
-                sex.printStackTrace();
+                Main.error(ex);
                 continue;
             }

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

@@ -27 +27 @@
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
+import java.util.TreeSet;
 
 import javax.swing.JFrame;
@@ -87 +89 @@
         mainFrame.setIconImages(l);
         mainFrame.addWindowListener(new WindowAdapter(){
-            @Override public void windowClosing(final WindowEvent arg0) {
+            @Override
+            public void windowClosing(final WindowEvent arg0) {
                 Main.exitJosm(true, 0);
             }
@@ -297 +300 @@
 
         Thread.setDefaultUncaughtExceptionHandler(new BugReportExceptionHandler());
-        // http://stuffthathappens.com/blog/2007/10/15/one-more-note-on-uncaught-exception-handlers/
+        // http://stackoverflow.com/q/75218/2257172
+        // To be replaced with official API when switching to Java 7: https://bugs.openjdk.java.net/browse/JDK-4714232
         System.setProperty("sun.awt.exception.handler", BugReportExceptionHandler.class.getName());
 
@@ -441 +445 @@
         public void run() {
 
-            // Handle proxy errors early to inform user he should change settings to be able to use JOSM correctly
-            handleProxyErrors();
+            // Handle proxy/network errors early to inform user he should change settings to be able to use JOSM correctly
+            if (!handleProxyErrors()) {
+                handleNetworkErrors();
+            }
 
             // Restore autosave layers after crash and start autosave thread
@@ -480 +486 @@
         }
 
-        private void handleProxyErrors() {
-            if (proxySelector.hasErrors()) {
+        private boolean handleNetworkOrProxyErrors(boolean hasErrors, String title, String message) {
+            if (hasErrors) {
                 ExtendedDialog ed = new ExtendedDialog(
-                        Main.parent, tr("Proxy errors occurred"),
+                        Main.parent, title,
                         new String[]{tr("Change proxy settings"), tr("Cancel")});
                 ed.setButtonIcons(new String[]{"dialogs/settings.png", "cancel.png"}).setCancelButton(2);
                 ed.setMinimumSize(new Dimension(460, 260));
                 ed.setIcon(JOptionPane.WARNING_MESSAGE);
-                ed.setContent(tr("JOSM tried to access the following resources:<br>" +
-                        "{0}" +
-                        "but <b>failed</b> to do so, because of the following proxy errors:<br>" +
-                        "{1}" +
-                        "Would you like to change your proxy settings now?",
-                        Utils.joinAsHtmlUnorderedList(proxySelector.getErrorResources()),
-                        Utils.joinAsHtmlUnorderedList(proxySelector.getErrorMessages())
-                ));
+                ed.setContent(message);
 
                 if (ed.showDialog().getValue() == 1) {
@@ -501 +500 @@
                 }
             }
+            return hasErrors;
+        }
+
+        private boolean handleProxyErrors() {
+            return handleNetworkOrProxyErrors(proxySelector.hasErrors(), tr("Proxy errors occurred"),
+                    tr("JOSM tried to access the following resources:<br>" +
+                            "{0}" +
+                            "but <b>failed</b> to do so, because of the following proxy errors:<br>" +
+                            "{1}" +
+                            "Would you like to change your proxy settings now?",
+                            Utils.joinAsHtmlUnorderedList(proxySelector.getErrorResources()),
+                            Utils.joinAsHtmlUnorderedList(proxySelector.getErrorMessages())
+                    ));
+        }
+
+        private boolean handleNetworkErrors() {
+            boolean condition = !networkErrors.isEmpty();
+            if (condition) {
+                Set<String> errors = new TreeSet<String>();
+                for (Throwable t : networkErrors.values()) {
+                    errors.add(t.toString());
+                }
+                return handleNetworkOrProxyErrors(condition, tr("Network errors occurred"),
+                        tr("JOSM tried to access the following resources:<br>" +
+                                "{0}" +
+                                "but <b>failed</b> to do so, because of the following network errors:<br>" +
+                                "{1}" +
+                                "It may result of a missing proxy configuration.<br>" +
+                                "Would you like to change your proxy settings now?",
+                                Utils.joinAsHtmlUnorderedList(networkErrors.keySet()),
+                                Utils.joinAsHtmlUnorderedList(errors)
+                        ));
+            }
+            return false;
         }
     }

trunk/src/org/openstreetmap/josm/io/MirroredInputStream.java

@@ -297 +297 @@
             con.setConnectTimeout(Main.pref.getInteger("socket.timeout.connect",15)*1000);
             con.setReadTimeout(Main.pref.getInteger("socket.timeout.read",30)*1000);
-            con.connect();
+            try {
+                con.connect();
+            } catch (IOException e) {
+                Main.addNetworkError(downloadUrl, Utils.getRootCause(e));
+                throw e;
+            }
             switch(con.getResponseCode()) {
             case HttpURLConnection.HTTP_OK:

trunk/src/org/openstreetmap/josm/io/OsmApi.java

@@ -78 +78 @@
     private static Map<String, OsmApi> instances = new HashMap<String, OsmApi>();
 
+    private URL url = null;
+
     /**
      * Replies the {@link OsmApi} for a given server URL
@@ -235 +237 @@
             /* This is an interim solution for openstreetmap.org not currently
              * transmitting their imagery blacklist in the capabilities call.
-             * remove this as soon as openstreetmap.org adds blacklists. */
+             * remove this as soon as openstreetmap.org adds blacklists.
+             * If you want to update this list, please ask for update of
+             * http://trac.openstreetmap.org/ticket/5024
+             * This list should not be maintained by each OSM editor (see #9210) */
             if (this.serverUrl.matches(".*openstreetmap.org/api.*") && capabilities.getImageryBlacklist().isEmpty())
             {
@@ -261 +266 @@
         } catch (OsmTransferCanceledException e) {
             throw e;
+        } catch (OsmTransferException e) {
+            initialized = false;
+            Main.addNetworkError(url, Utils.getRootCause(e));
+            throw new OsmApiInitializationException(e);
         } catch (Exception e) {
             initialized = false;
@@ -598 +607 @@
         while(true) { // the retry loop
             try {
-                URL url = new URL(new URL(getBaseUrl()), urlSuffix);
-                System.out.print(requestMethod + " " + url + "... ");
+                url = new URL(new URL(getBaseUrl()), urlSuffix);
+                Main.info(requestMethod + " " + url + "... ");
                 // fix #5369, see http://www.tikalk.com/java/forums/httpurlconnection-disable-keep-alive
                 activeConnection = Utils.openHttpConnection(url, false);

trunk/src/org/openstreetmap/josm/tools/Utils.java

@@ -419 +419 @@
             return true;
         } catch (IllegalStateException ex) {
-            ex.printStackTrace();
+            Main.error(ex);
             return false;
         }
@@ -449 +449 @@
             }
         } catch (UnsupportedFlavorException ex) {
-            ex.printStackTrace();
+            Main.error(ex);
             return null;
         } catch (IOException ex) {
-            ex.printStackTrace();
+            Main.error(ex);
             return null;
         }
@@ -895 +895 @@
     }
 
+    /**
+     * Returns the root cause of a throwable object.
+     * @param t The object to get root cause for
+     * @return the root cause of {@code t}
+     * @since 6639
+     */
+    public static Throwable getRootCause(Throwable t) {
+        Throwable result = t;
+        if (result != null) {
+            Throwable cause = result.getCause();
+            while (cause != null && cause != result) {
+                result = cause;
+                cause = result.getCause();
+            }
+        }
+        return result;
+    }
 }

trunk/src/org/openstreetmap/josm/tools/WikiReader.java

@@ -18 +18 @@
     private final String baseurl;
 
+    /**
+     * Constructs a new {@code WikiReader} for the given base URL.
+     * @param baseurl The wiki base URL
+     */
     public WikiReader(String baseurl) {
         this.baseurl = baseurl;
@@ -80 +84 @@
 
     private String readLang(URL url) throws IOException {
-        BufferedReader in = Utils.openURLReader(url);
+        BufferedReader in;
+        try {
+            in = Utils.openURLReader(url);
+        } catch (IOException e) {
+            Main.addNetworkError(url, Utils.getRootCause(e));
+            throw e;
+        }
         try {
             return readFromTrac(in, url);
@@ -124 +134 @@
                 // add a border="0" attribute to images, otherwise the internal help browser
                 // will render a thick  border around images inside an <a> element
-                //
                 b += line.replaceAll("<img ", "<img border=\"0\" ")
                          .replaceAll("<span class=\"icon\">.</span>", "")