Index: trunk/src/org/openstreetmap/josm/actions/downloadtasks/DownloadTask.java =================================================================== --- trunk/src/org/openstreetmap/josm/actions/downloadtasks/DownloadTask.java (revision 17326) +++ trunk/src/org/openstreetmap/josm/actions/downloadtasks/DownloadTask.java (revision 17330) @@ -4,9 +4,12 @@ import java.net.URL; import java.util.List; +import java.util.Objects; import java.util.concurrent.Future; +import java.util.stream.Collectors; import org.openstreetmap.josm.data.Bounds; import org.openstreetmap.josm.gui.progress.NullProgressMonitor; import org.openstreetmap.josm.gui.progress.ProgressMonitor; +import org.openstreetmap.josm.tools.ExceptionUtil; /** @@ -116,4 +119,22 @@ /** + * Replies the error messages of the task. Empty list, if no error messages are available. + * + * @return the list of error messages + * @since 17330 + */ + default List getErrorMessages() { + return getErrorObjects().stream().map(o -> { + if (o instanceof String) { + return (String) o; + } else if (o instanceof Exception) { + return ExceptionUtil.explainException((Exception) o).replace("", "").replace("", ""); + } else { + return (String) null; + } + }).filter(Objects::nonNull).collect(Collectors.toList()); + } + + /** * Cancels the asynchronous download task. * Index: trunk/src/org/openstreetmap/josm/actions/downloadtasks/DownloadTaskList.java =================================================================== --- trunk/src/org/openstreetmap/josm/actions/downloadtasks/DownloadTaskList.java (revision 17326) +++ trunk/src/org/openstreetmap/josm/actions/downloadtasks/DownloadTaskList.java (revision 17330) @@ -9,5 +9,4 @@ import java.awt.geom.Area; import java.awt.geom.Rectangle2D; -import java.util.ArrayList; import java.util.Collection; import java.util.LinkedHashSet; @@ -35,5 +34,4 @@ import org.openstreetmap.josm.gui.progress.ProgressMonitor; import org.openstreetmap.josm.gui.util.GuiHelper; -import org.openstreetmap.josm.tools.ExceptionUtil; import org.openstreetmap.josm.tools.ImageProvider; import org.openstreetmap.josm.tools.Logging; @@ -247,22 +245,13 @@ } } - Set errors = tasks.stream().flatMap(t -> t.getErrorObjects().stream()).collect(Collectors.toSet()); + Set errors = tasks.stream().flatMap(t -> t.getErrorMessages().stream()).collect(Collectors.toSet()); if (!errors.isEmpty()) { - final Collection items = new ArrayList<>(); - for (Object error : errors) { - if (error instanceof String) { - items.add((String) error); - } else if (error instanceof Exception) { - items.add(ExceptionUtil.explainException((Exception) error)); - } - } - GuiHelper.runInEDT(() -> { - if (items.size() == 1 && PostDownloadHandler.isNoDataErrorMessage(items.iterator().next())) { - new Notification(items.iterator().next()).setIcon(JOptionPane.WARNING_MESSAGE).show(); + if (errors.size() == 1 && PostDownloadHandler.isNoDataErrorMessage(errors.iterator().next())) { + new Notification(errors.iterator().next()).setIcon(JOptionPane.WARNING_MESSAGE).show(); } else { JOptionPane.showMessageDialog(MainApplication.getMainFrame(), "" + tr("The following errors occurred during mass download: {0}", - Utils.joinAsHtmlUnorderedList(items)) + "", + Utils.joinAsHtmlUnorderedList(errors)) + "", tr("Errors during download"), JOptionPane.ERROR_MESSAGE); return; Index: trunk/src/org/openstreetmap/josm/actions/downloadtasks/PostDownloadHandler.java =================================================================== --- trunk/src/org/openstreetmap/josm/actions/downloadtasks/PostDownloadHandler.java (revision 17326) +++ trunk/src/org/openstreetmap/josm/actions/downloadtasks/PostDownloadHandler.java (revision 17330) @@ -5,5 +5,4 @@ import java.awt.GraphicsEnvironment; -import java.util.ArrayList; import java.util.Collection; import java.util.HashSet; @@ -22,5 +21,4 @@ import org.openstreetmap.josm.gui.Notification; import org.openstreetmap.josm.gui.util.GuiHelper; -import org.openstreetmap.josm.tools.ExceptionUtil; import org.openstreetmap.josm.tools.Logging; import org.openstreetmap.josm.tools.Utils; @@ -126,21 +124,10 @@ // multiple error object? prepare a HTML list // - if (!errors.isEmpty()) { - final Collection items = new ArrayList<>(); - for (Object error : errors) { - if (error instanceof String) { - items.add((String) error); - } else if (error instanceof Exception) { - items.add(ExceptionUtil.explainException((Exception) error)); - } - } - - if (!GraphicsEnvironment.isHeadless()) { - SwingUtilities.invokeLater(() -> JOptionPane.showMessageDialog( - MainApplication.getMainFrame(), - ""+Utils.joinAsHtmlUnorderedList(items)+"", - tr("Errors during download"), - JOptionPane.ERROR_MESSAGE)); - } + if (!errors.isEmpty() && !GraphicsEnvironment.isHeadless()) { + SwingUtilities.invokeLater(() -> JOptionPane.showMessageDialog( + MainApplication.getMainFrame(), + ""+Utils.joinAsHtmlUnorderedList(task.getErrorMessages())+"", + tr("Errors during download"), + JOptionPane.ERROR_MESSAGE)); } } Index: trunk/src/org/openstreetmap/josm/io/remotecontrol/RequestProcessor.java =================================================================== --- trunk/src/org/openstreetmap/josm/io/remotecontrol/RequestProcessor.java (revision 17326) +++ trunk/src/org/openstreetmap/josm/io/remotecontrol/RequestProcessor.java (revision 17330) @@ -43,4 +43,5 @@ import org.openstreetmap.josm.io.remotecontrol.handler.RequestHandler.RequestHandlerErrorException; import org.openstreetmap.josm.io.remotecontrol.handler.RequestHandler.RequestHandlerForbiddenException; +import org.openstreetmap.josm.io.remotecontrol.handler.RequestHandler.RequestHandlerOsmApiException; import org.openstreetmap.josm.io.remotecontrol.handler.VersionHandler; import org.openstreetmap.josm.tools.Logging; @@ -185,5 +186,5 @@ String get = in.readLine(); if (get == null) { - sendError(out); + sendInternalError(out, null); return; } @@ -192,10 +193,10 @@ StringTokenizer st = new StringTokenizer(get); if (!st.hasMoreTokens()) { - sendError(out); + sendInternalError(out, null); return; } String method = st.nextToken(); if (!st.hasMoreTokens()) { - sendError(out); + sendInternalError(out, null); return; } @@ -252,11 +253,5 @@ String help = "No command specified! The following commands are available:
    " + usage + "
" + "See "+websiteDoc+" for complete documentation."; - sendHeader(out, "400 Bad Request", "text/html", true); - out.write(String.format( - RESPONSE_TEMPLATE, - "Bad Request", - "

HTTP Error 400: Bad Request

" + - "

" + help + "

")); - out.flush(); + sendBadRequest(out, help); } else { // create handler object @@ -273,7 +268,10 @@ out.write(handler.getContent()); out.flush(); + } catch (RequestHandlerOsmApiException ex) { + Logging.debug(ex); + sendBadGateway(out, ex.getMessage()); } catch (RequestHandlerErrorException ex) { Logging.debug(ex); - sendError(out); + sendInternalError(out, ex.getMessage()); } catch (RequestHandlerBadRequestException ex) { Logging.debug(ex); @@ -289,5 +287,5 @@ Logging.error(e); try { - sendError(out); + sendInternalError(out, e.getMessage()); } catch (IOException e1) { Logging.warn(e1); @@ -302,18 +300,11 @@ } - /** - * Sends a 500 error: server error - * - * @param out - * The writer where the error is written - * @throws IOException - * If the error can not be written - */ - private static void sendError(Writer out) throws IOException { - sendHeader(out, "500 Internal Server Error", "text/html", true); + private static void sendError(Writer out, int errorCode, String errorName, String help) throws IOException { + sendHeader(out, errorCode + " " + errorName, "text/html", true); out.write(String.format( RESPONSE_TEMPLATE, - "Internal Error", - "

HTTP Error 500: Internal Server Error

" + "" + errorName + "", + "

HTTP Error " + errorCode + ": " + errorName + "

" + + (help == null ? "" : "

"+Utils.escapeReservedCharactersHTML(help) + "

") )); out.flush(); @@ -321,23 +312,5 @@ /** - * Sends a 501 error: not implemented - * - * @param out - * The writer where the error is written - * @throws IOException - * If the error can not be written - */ - private static void sendNotImplemented(Writer out) throws IOException { - sendHeader(out, "501 Not Implemented", "text/html", true); - out.write(String.format( - RESPONSE_TEMPLATE, - "Not Implemented", - "

HTTP Error 501: Not Implemented

" - )); - out.flush(); - } - - /** - * Sends a 403 error: forbidden + * Sends a 500 error: internal server error * * @param out @@ -348,13 +321,46 @@ * If the error can not be written */ + private static void sendInternalError(Writer out, String help) throws IOException { + sendError(out, 500, "Internal Server Error", help); + } + + /** + * Sends a 501 error: not implemented + * + * @param out + * The writer where the error is written + * @throws IOException + * If the error can not be written + */ + private static void sendNotImplemented(Writer out) throws IOException { + sendError(out, 501, "Not Implemented", null); + } + + /** + * Sends a 502 error: bad gateway + * + * @param out + * The writer where the error is written + * @param help + * Optional HTML help content to display, can be null + * @throws IOException + * If the error can not be written + */ + private static void sendBadGateway(Writer out, String help) throws IOException { + sendError(out, 502, "Bad Gateway", help); + } + + /** + * Sends a 403 error: forbidden + * + * @param out + * The writer where the error is written + * @param help + * Optional HTML help content to display, can be null + * @throws IOException + * If the error can not be written + */ private static void sendForbidden(Writer out, String help) throws IOException { - sendHeader(out, "403 Forbidden", "text/html", true); - out.write(String.format( - RESPONSE_TEMPLATE, - "Forbidden", - "

HTTP Error 403: Forbidden

" + - (help == null ? "" : "

"+Utils.escapeReservedCharactersHTML(help) + "

") - )); - out.flush(); + sendError(out, 403, "Forbidden", help); } @@ -367,12 +373,5 @@ */ private static void sendBadRequest(Writer out, String help) throws IOException { - sendHeader(out, "400 Bad Request", "text/html", true); - out.write(String.format( - RESPONSE_TEMPLATE, - "Bad Request", - "

HTTP Error 400: Bad Request

" + - (help == null ? "" : ("

" + Utils.escapeReservedCharactersHTML(help) + "

")) - )); - out.flush(); + sendError(out, 400, "Bad Request", help); } Index: trunk/src/org/openstreetmap/josm/io/remotecontrol/handler/LoadAndZoomHandler.java =================================================================== --- trunk/src/org/openstreetmap/josm/io/remotecontrol/handler/LoadAndZoomHandler.java (revision 17326) +++ trunk/src/org/openstreetmap/josm/io/remotecontrol/handler/LoadAndZoomHandler.java (revision 17330) @@ -12,5 +12,8 @@ import java.util.Map; import java.util.Set; +import java.util.concurrent.ExecutionException; import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; import javax.swing.JOptionPane; @@ -20,5 +23,4 @@ import org.openstreetmap.josm.actions.downloadtasks.DownloadOsmTask; import org.openstreetmap.josm.actions.downloadtasks.DownloadParams; -import org.openstreetmap.josm.actions.downloadtasks.DownloadTask; import org.openstreetmap.josm.actions.downloadtasks.PostDownloadHandler; import org.openstreetmap.josm.data.Bounds; @@ -36,4 +38,5 @@ import org.openstreetmap.josm.gui.Notification; import org.openstreetmap.josm.gui.util.GuiHelper; +import org.openstreetmap.josm.io.OsmApiException; import org.openstreetmap.josm.io.remotecontrol.AddTagsDialog; import org.openstreetmap.josm.io.remotecontrol.PermissionPrefWithDefault; @@ -120,5 +123,5 @@ @Override protected void handleRequest() throws RequestHandlerErrorException { - DownloadTask osmTask = new DownloadOsmTask(); + DownloadOsmTask osmTask = new DownloadOsmTask(); try { DownloadParams settings = getDownloadParams(); @@ -154,9 +157,16 @@ Future future = osmTask.download(settings, new Bounds(minlat, minlon, maxlat, maxlon), null /* let the task manage the progress monitor */); - MainApplication.worker.submit(new PostDownloadHandler(osmTask, future)); + MainApplication.worker.submit(new PostDownloadHandler(osmTask, future)) + .get(OSM_DOWNLOAD_TIMEOUT.get(), TimeUnit.SECONDS); + if (osmTask.isFailed()) { + Object error = osmTask.getErrorObjects().get(0); + throw error instanceof OsmApiException + ? new RequestHandlerOsmApiException((OsmApiException) error) + : new RequestHandlerErrorException(String.join(", ", osmTask.getErrorMessages())); + } } } } - } catch (RuntimeException ex) { // NOPMD + } catch (RuntimeException | InterruptedException | ExecutionException | TimeoutException ex) { // NOPMD Logging.warn("RemoteControl: Error parsing load_and_zoom remote control request:"); Logging.error(ex); Index: trunk/src/org/openstreetmap/josm/io/remotecontrol/handler/LoadObjectHandler.java =================================================================== --- trunk/src/org/openstreetmap/josm/io/remotecontrol/handler/LoadObjectHandler.java (revision 17326) +++ trunk/src/org/openstreetmap/josm/io/remotecontrol/handler/LoadObjectHandler.java (revision 17330) @@ -103,4 +103,7 @@ } } + if (ps.isEmpty()) { + throw new RequestHandlerBadRequestException(tr("No valid object identifier has been provided")); + } } } Index: trunk/src/org/openstreetmap/josm/io/remotecontrol/handler/RequestHandler.java =================================================================== --- trunk/src/org/openstreetmap/josm/io/remotecontrol/handler/RequestHandler.java (revision 17326) +++ trunk/src/org/openstreetmap/josm/io/remotecontrol/handler/RequestHandler.java (revision 17330) @@ -25,5 +25,7 @@ import org.openstreetmap.josm.data.osm.UploadPolicy; import org.openstreetmap.josm.data.preferences.BooleanProperty; +import org.openstreetmap.josm.data.preferences.IntegerProperty; import org.openstreetmap.josm.gui.MainApplication; +import org.openstreetmap.josm.io.OsmApiException; import org.openstreetmap.josm.io.remotecontrol.PermissionPrefWithDefault; import org.openstreetmap.josm.spi.preferences.Config; @@ -43,4 +45,6 @@ /** preference to determine if remote control loads data in a new layer */ public static final BooleanProperty LOAD_IN_NEW_LAYER = new BooleanProperty("remotecontrol.new-layer", false); + /** preference to define OSM download timeout in seconds */ + public static final IntegerProperty OSM_DOWNLOAD_TIMEOUT = new IntegerProperty("remotecontrol.osm.download.timeout", 5*60); protected static final Pattern SPLITTER_COMMA = Pattern.compile(",\\s*"); @@ -416,7 +420,31 @@ /** * Constructs a new {@code RequestHandlerErrorException}. + * @param message the detail message. The detail message is saved for later retrieval by the {@link #getMessage()} method. + * @since 17330 + */ + public RequestHandlerErrorException(String message) { + super(message); + } + + /** + * Constructs a new {@code RequestHandlerErrorException}. * @param cause the cause (which is saved for later retrieval by the {@link #getCause()} method). */ public RequestHandlerErrorException(Throwable cause) { + super(cause); + } + } + + /** + * Error raised for OSM API errors. + * @since 17330 + */ + public static class RequestHandlerOsmApiException extends RequestHandlerErrorException { + + /** + * Constructs a new {@code RequestHandlerOsmApiException}. + * @param cause the cause (which is saved for later retrieval by the {@link #getCause()} method). + */ + public RequestHandlerOsmApiException(OsmApiException cause) { super(cause); }