Ticket #22680: 22680.patch

src/org/openstreetmap/josm/io/OsmJsonReader.java

@@ -4 +4 @@
 import static org.openstreetmap.josm.tools.I18n.tr;
 
 import java.io.InputStream;
+import java.net.SocketException;
 import java.util.Collection;
 import java.util.Map.Entry;
 
 import javax.json.Json;
 import javax.json.JsonArray;
+import javax.json.JsonException;
 import javax.json.JsonNumber;
 import javax.json.JsonObject;
 import javax.json.JsonString;
 import javax.json.JsonValue;
 import javax.json.stream.JsonParser;
 import javax.json.stream.JsonParser.Event;
+import javax.json.stream.JsonParsingException;
 
 import org.openstreetmap.josm.data.osm.DataSet;
 import org.openstreetmap.josm.data.osm.PrimitiveData;
@@ -178 +181 @@
 
     @Override
     protected DataSet doParseDataSet(InputStream source, ProgressMonitor progressMonitor) throws IllegalDataException {
-        return doParseDataSet(source, progressMonitor, ir -> {
-            setParser(Json.createParser(ir));
-            parse();
-        });
+        try {
+            return doParseDataSet(source, progressMonitor, ir -> {
+                setParser(Json.createParser(ir));
+                parse();
+            });
+        } catch (JsonParsingException exception) {
+            throw new IllegalDataException(exception);
+        } catch (JsonException exception) {
+            if (exception.getCause() instanceof SocketException) {
+                SocketException soe = (SocketException) exception.getCause();
+                soe.addSuppressed(exception); // Add the caught exception as a suppressed exception
+                throw new IllegalDataException(soe); // NOPMD -- PreserveStackTrace should be fixed with PMD 7
+            }
+            throw exception;
+        }
     }
 
     /**

test/unit/org/openstreetmap/josm/io/OsmJsonReaderTest.java

@@ -1 +1 @@
 // License: GPL. For details, see LICENSE file.
 package org.openstreetmap.josm.io;
 
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
 import java.io.ByteArrayInputStream;
+import java.io.IOException;
 import java.io.InputStream;
+import java.net.SocketException;
 import java.nio.charset.StandardCharsets;
 import java.time.Instant;
 import java.util.Iterator;
+import java.util.concurrent.atomic.AtomicBoolean;
 
+import javax.json.JsonException;
+
+import org.junit.jupiter.api.Test;
 import org.openstreetmap.josm.data.coor.LatLon;
 import org.openstreetmap.josm.data.osm.DataSet;
 import org.openstreetmap.josm.data.osm.Node;
@@ -19 +27 @@
 import org.openstreetmap.josm.data.osm.Way;
 import org.openstreetmap.josm.gui.progress.NullProgressMonitor;
 import org.openstreetmap.josm.testutils.annotations.BasicPreferences;
-
-import org.junit.jupiter.api.Test;
+import org.openstreetmap.josm.tools.ExceptionUtil;
 
 /**
  * Unit tests of {@link OsmReader} class.
@@ -246 +253 @@
                 "  \"remark\": \"runtime error: Query ran out of memory in \\\"query\\\" at line 5.\"\n");
         assertEquals("runtime error: Query ran out of memory in \"query\" at line 5.", ds.getRemark());
     }
+
+
+    /**
+     * See #22680: Unexpected exception downloading from Overpass query
+     * The JSON parser throws {@link RuntimeException}s, specifically
+     * <ul>
+     *     <li>{@link javax.json.JsonException}</li>
+     *     <li>{@link javax.json.stream.JsonParsingException}, extends {@link javax.json.JsonException}</li>
+     *     <li>{@link javax.json.stream.JsonGenerationException}, extends {@link javax.json.JsonException}
+     *         (which we don't care about when we are <em>parsing</em> JSON)</li>
+     * </ul>
+     */
+    @SuppressWarnings("resource")
+    @Test
+    void testException() {
+        final ByteArrayInputStream bais =
+                new ByteArrayInputStream("{\"type\", \"node\", \"id\": 1, \"lat\": 1.0, \"lon\": 2.0}".getBytes(StandardCharsets.UTF_8));
+        final AtomicBoolean throwJson = new AtomicBoolean();
+        final InputStream socketExceptionStream = new InputStream() {
+            int read = 0; // Necessary, since otherwise the exception might not be wrapped in a Json exception
+            @Override
+            public int read() throws IOException {
+                try {
+                    if (read > 0 && !throwJson.get()) {
+                        throw new SocketException("Read timed out");
+                    } else if (read > 0 && throwJson.get()) {
+                        throw new JsonException("Some random json exception");
+                    }
+                    return bais.read();
+                } finally {
+                    read++;
+                }
+            }
+        };
+        // Check that a SocketException is properly reported
+        IllegalDataException ide = assertThrows(IllegalDataException.class,
+                () -> OsmJsonReader.parseDataSet(socketExceptionStream, NullProgressMonitor.INSTANCE));
+        assertEquals("java.net.SocketException: Read timed out", ExceptionUtil.explainException(ide));
+        assertDoesNotThrow(socketExceptionStream::close);
+        bais.reset();
+        // Check that a generic exception is properly thrown -- we only want to handle known "good" cases specially
+        throwJson.set(true);
+        assertThrows(JsonException.class, () -> OsmJsonReader.parseDataSet(socketExceptionStream, NullProgressMonitor.INSTANCE));
+        bais.reset();
+        // Check that a generic parsing error is properly reported
+        ide = assertThrows(IllegalDataException.class, () -> OsmJsonReader.parseDataSet(bais, NullProgressMonitor.INSTANCE));
+        assertEquals("javax.json.stream.JsonParsingException: Invalid token=COMMA at (line no=1, column no=8, offset=7). " +
+                "Expected tokens are: [COLON]", ExceptionUtil.explainException(ide));
+        bais.reset();
+        // Check that an unknown exception is thrown properly
+    }
 }