- Timestamp:
- 2022-09-06T18:40:21+02:00 (2 years ago)
- Location:
- trunk
- Files:
-
- 4 edited
Legend:
- Unmodified
- Added
- Removed
-
trunk/src/org/openstreetmap/josm/gui/progress/swing/ProgressMonitorExecutor.java
r18150 r18549 11 11 import org.openstreetmap.josm.tools.Logging; 12 12 import org.openstreetmap.josm.tools.Utils; 13 import org.openstreetmap.josm.tools.bugreport.BugReport; 13 14 14 15 /** … … 59 60 if (t != null) { 60 61 Logging.error("Thread {0} raised {1}", Thread.currentThread().getName(), t); 62 BugReport.addSuppressedException(t); 61 63 } 62 64 } -
trunk/src/org/openstreetmap/josm/gui/util/GuiHelper.java
r18365 r18549 212 212 static void handleEDTException(Throwable t) { 213 213 Logging.logWithStackTrace(Logging.LEVEL_ERROR, t, "Exception raised in EDT"); 214 BugReport.addSuppressedException(t); 214 215 } 215 216 -
trunk/src/org/openstreetmap/josm/tools/bugreport/BugReport.java
r15735 r18549 5 5 import java.io.Serializable; 6 6 import java.io.StringWriter; 7 import java.time.Instant; 8 import java.util.ArrayDeque; 9 import java.util.Deque; 7 10 import java.util.concurrent.CopyOnWriteArrayList; 8 11 import java.util.function.Predicate; 12 13 import org.openstreetmap.josm.tools.Pair; 9 14 10 15 /** … … 40 45 public final class BugReport implements Serializable { 41 46 private static final long serialVersionUID = 1L; 47 /** The maximum suppressed exceptions to keep to report */ 48 private static final byte MAXIMUM_SUPPRESSED_EXCEPTIONS = 4; 49 /** The list of suppressed exceptions, Pair<time reported, exception> */ 50 private static final Deque<Pair<Instant, Throwable>> SUPPRESSED_EXCEPTIONS = new ArrayDeque<>(MAXIMUM_SUPPRESSED_EXCEPTIONS); 42 51 43 52 private boolean includeStatusReport = true; … … 54 63 this.exception = e; 55 64 includeAllStackTraces = e.mayHaveConcurrentSource(); 65 } 66 67 /** 68 * Add a suppressed exception. Mostly useful for when a chain of exceptions causes an actual bug report. 69 * This should only be used when an exception is raised in {@link org.openstreetmap.josm.gui.util.GuiHelper} 70 * or {@link org.openstreetmap.josm.gui.progress.swing.ProgressMonitorExecutor} at this time. 71 * {@link org.openstreetmap.josm.tools.Logging} may call this in the future, when logging a throwable. 72 * @param t The throwable raised. If {@code null}, we add a new {@code NullPointerException} instead. 73 * @since 18549 74 */ 75 public static void addSuppressedException(Throwable t) { 76 SUPPRESSED_EXCEPTIONS.add(new Pair<>(Instant.now(), t != null ? t : new NullPointerException())); 77 // Ensure we don't call pop in more than MAXIMUM_SUPPRESSED_EXCEPTIONS threads. This guard is 78 // here just in case someone doesn't read the javadocs. 79 synchronized (SUPPRESSED_EXCEPTIONS) { 80 // Ensure we aren't keeping exceptions forever 81 while (SUPPRESSED_EXCEPTIONS.size() > MAXIMUM_SUPPRESSED_EXCEPTIONS) { 82 SUPPRESSED_EXCEPTIONS.pop(); 83 } 84 } 56 85 } 57 86 … … 136 165 exception.printReportThreadsTo(out); 137 166 } 167 synchronized (SUPPRESSED_EXCEPTIONS) { 168 if (!SUPPRESSED_EXCEPTIONS.isEmpty()) { 169 out.println("=== ADDITIONAL EXCEPTIONS ==="); 170 // Avoid multiple bug reports from reading from the deque at the same time. 171 while (SUPPRESSED_EXCEPTIONS.peek() != null) { 172 Pair<Instant, Throwable> currentException = SUPPRESSED_EXCEPTIONS.pop(); 173 out.println("==== Exception at " + currentException.a.toEpochMilli() + " ===="); 174 currentException.b.printStackTrace(out); 175 } 176 } 177 } 138 178 return stringWriter.toString().replaceAll("\r", ""); 139 179 } -
trunk/test/unit/org/openstreetmap/josm/tools/bugreport/BugReportTest.java
r18037 r18549 2 2 package org.openstreetmap.josm.tools.bugreport; 3 3 4 import static org.junit.jupiter.api.Assertions.assertAll; 5 import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; 4 6 import static org.junit.jupiter.api.Assertions.assertEquals; 7 import static org.junit.jupiter.api.Assertions.assertFalse; 5 8 import static org.junit.jupiter.api.Assertions.assertSame; 6 9 import static org.junit.jupiter.api.Assertions.assertTrue; … … 9 12 import java.io.PrintWriter; 10 13 import java.io.StringWriter; 11 14 import java.lang.reflect.Field; 15 import java.util.function.Consumer; 16 import java.util.logging.Handler; 17 import java.util.regex.Matcher; 18 import java.util.regex.Pattern; 19 import java.util.stream.Stream; 20 21 import org.junit.jupiter.api.AfterAll; 22 import org.junit.jupiter.api.BeforeAll; 12 23 import org.junit.jupiter.api.Test; 24 import org.junit.jupiter.params.ParameterizedTest; 25 import org.junit.jupiter.params.provider.Arguments; 26 import org.junit.jupiter.params.provider.MethodSource; 27 import org.junit.platform.commons.util.ReflectionUtils; 13 28 import org.openstreetmap.josm.actions.ShowStatusReportAction; 29 import org.openstreetmap.josm.gui.MainApplication; 30 import org.openstreetmap.josm.gui.util.GuiHelper; 14 31 import org.openstreetmap.josm.testutils.annotations.BasicPreferences; 32 import org.openstreetmap.josm.tools.Logging; 15 33 16 34 /** … … 21 39 @BasicPreferences 22 40 class BugReportTest { 41 private static Handler[] handlers; 42 43 @AfterAll 44 static void cleanup() { 45 // Clear queue 46 new BugReport(BugReport.intercept(new NullPointerException())).getReportText(""); 47 Logging.clearLastErrorAndWarnings(); 48 for (Handler handler : handlers) { 49 Logging.getLogger().addHandler(handler); 50 } 51 } 52 53 @BeforeAll 54 static void setup() { 55 handlers = Logging.getLogger().getHandlers(); 56 // Avoid console spam 57 for (Handler handler : handlers) { 58 Logging.getLogger().removeHandler(handler); 59 } 60 } 61 23 62 /** 24 63 * Test {@link BugReport#getReportText} … … 70 109 return BugReport.getCallingMethod(2); 71 110 } 111 112 @Test 113 void testSuppressedExceptionsOrder() { 114 final String methodName = "testSuppressedExceptionsOrder"; 115 BugReport.addSuppressedException(new NullPointerException(methodName)); 116 BugReport.addSuppressedException(new IllegalStateException(methodName)); 117 BugReport bugReport = new BugReport(BugReport.intercept(new IOException(methodName))); 118 final String report = assertDoesNotThrow(() -> bugReport.getReportText(methodName)); 119 assertAll(() -> assertTrue(report.contains("NullPointerException")), 120 () -> assertTrue(report.contains("IOException")), 121 () -> assertTrue(report.contains("IllegalStateException"))); 122 int ioe = report.indexOf("IOException"); 123 int npe = report.indexOf("NullPointerException"); 124 int ise = report.indexOf("IllegalStateException"); 125 assertAll("Ordering of exceptions is wrong", 126 () -> assertTrue(ioe < npe, "IOException should be reported before NullPointerException"), 127 () -> assertTrue(npe < ise, "NullPointerException should be reported before IllegalStateException")); 128 } 129 130 static Stream<Arguments> testSuppressedExceptions() { 131 return Stream.of( 132 Arguments.of("GuiHelper::runInEDTAndWaitAndReturn", 133 (Consumer<Runnable>) r -> GuiHelper.runInEDTAndWaitAndReturn(() -> { 134 r.run(); 135 return null; 136 })), 137 Arguments.of("GuiHelper::runInEDTAndWait", (Consumer<Runnable>) GuiHelper::runInEDTAndWait), 138 Arguments.of("MainApplication.worker", (Consumer<Runnable>) MainApplication.worker::execute) 139 ); 140 } 141 142 @ParameterizedTest 143 @MethodSource 144 void testSuppressedExceptions(String workerName, Consumer<Runnable> worker) { 145 // Throw a npe in the worker. Workers might give us the exception, wrapped or otherwise. 146 try { 147 worker.accept(() -> { 148 throw new NullPointerException(); 149 }); 150 } catch (Exception e) { 151 // pass. MainApplication.worker can continue throwing the NPE; 152 Logging.trace(e); 153 } 154 // Ensure that the threads are synced 155 assertDoesNotThrow(() -> worker.accept(() -> { /* sync */ })); 156 // Now throw an exception 157 BugReport bugReport = new BugReport(BugReport.intercept(new IOException("testSuppressedExceptions"))); 158 String report = bugReport.getReportText(workerName); 159 assertTrue(report.contains("IOException")); 160 assertTrue(report.contains("NullPointerException")); 161 } 162 163 @Test 164 void testSuppressedExceptionsReportedOnce() { 165 // Add the exception 166 BugReport.addSuppressedException(new NullPointerException("testSuppressedExceptionsReportedOnce")); 167 BugReport bugReport = new BugReport(BugReport.intercept(new IOException("testSuppressedExceptionsReportedOnce"))); 168 // Get the report which clears the suppressed exceptions 169 String report = bugReport.getReportText(""); 170 assertTrue(report.contains("IOException")); 171 assertTrue(report.contains("NullPointerException")); 172 173 BugReport bugReport2 = new BugReport(BugReport.intercept(new IOException("testSuppressedExceptionsReportedOnce"))); 174 String report2 = bugReport2.getReportText(""); 175 assertTrue(report2.contains("IOException")); 176 assertFalse(report2.contains("NullPointerException")); 177 } 178 179 @Test 180 void testManyExceptions() throws ReflectiveOperationException { 181 Field suppressedExceptions = BugReport.class.getDeclaredField("MAXIMUM_SUPPRESSED_EXCEPTIONS"); 182 ReflectionUtils.makeAccessible(suppressedExceptions); 183 final byte expected = suppressedExceptions.getByte(null); 184 final int end = 2 * expected; 185 // Add many suppressed exceptions 186 for (int i = 0; i < end; i++) { 187 BugReport.addSuppressedException(new NullPointerException("NPE: " + i)); 188 } 189 BugReport bugReport = new BugReport(BugReport.intercept(new IOException("testManyExceptions"))); 190 String report = bugReport.getReportText(""); 191 Matcher matcher = Pattern.compile("NPE: (\\d+)").matcher(report); 192 for (int i = end - expected; i < end; ++i) { 193 assertTrue(matcher.find()); 194 assertEquals(Integer.toString(i), matcher.group(1)); 195 } 196 assertFalse(matcher.find()); 197 } 198 199 @Test 200 void testNullException() { 201 // This should add a NPE to the suppressed exceptions 202 assertDoesNotThrow(() -> BugReport.addSuppressedException(null)); 203 BugReport bugReport = new BugReport(BugReport.intercept(new IOException("testNullException"))); 204 // Getting the report text should not throw an exception. 205 String report = assertDoesNotThrow(() -> bugReport.getReportText("")); 206 assertTrue(report.contains("IOException")); 207 assertTrue(report.contains("NullPointerException")); 208 } 72 209 }
Note:
See TracChangeset
for help on using the changeset viewer.