From a530221213815540a03d34a71e880db2357bc61e Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Tue, 10 Nov 2015 00:09:28 -0800 Subject: [PATCH] Ensure startup failures are only logged once Update SpringApplication so that startup exceptions are only logged once. A custom UncaughtExceptionHandler is now used when running in the main thread to suppress errors that have already been logged. Fixes gh-4423 --- .../FileWatchingFailureHandler.java | 1 - .../devtools/restart/RestartLauncher.java | 1 + .../boot/devtools/restart/Restarter.java | 5 +- .../boot/LoggedExceptionHandler.java | 82 +++++++++++++++++++ .../boot/SpringApplication.java | 49 ++++++++--- .../boot/SpringApplicationTests.java | 29 +++++++ 6 files changed, 151 insertions(+), 16 deletions(-) create mode 100644 spring-boot/src/main/java/org/springframework/boot/LoggedExceptionHandler.java diff --git a/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/autoconfigure/FileWatchingFailureHandler.java b/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/autoconfigure/FileWatchingFailureHandler.java index 4bc6fa130b..4eb5cdb6c5 100644 --- a/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/autoconfigure/FileWatchingFailureHandler.java +++ b/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/autoconfigure/FileWatchingFailureHandler.java @@ -42,7 +42,6 @@ class FileWatchingFailureHandler implements FailureHandler { @Override public Outcome handle(Throwable failure) { - failure.printStackTrace(); CountDownLatch latch = new CountDownLatch(1); FileSystemWatcher watcher = this.fileSystemWatcherFactory.getFileSystemWatcher(); watcher.addSourceFolders( diff --git a/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/restart/RestartLauncher.java b/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/restart/RestartLauncher.java index 7020fe2eec..131db1240f 100644 --- a/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/restart/RestartLauncher.java +++ b/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/restart/RestartLauncher.java @@ -50,6 +50,7 @@ class RestartLauncher extends Thread { } catch (Throwable ex) { this.error = ex; + getUncaughtExceptionHandler().uncaughtException(this, ex); } } diff --git a/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/restart/Restarter.java b/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/restart/Restarter.java index c08175c796..e59df1c4d9 100644 --- a/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/restart/Restarter.java +++ b/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/restart/Restarter.java @@ -271,10 +271,7 @@ public class Restarter { return; } if (failureHandler.handle(error) == Outcome.ABORT) { - if (error instanceof Exception) { - throw (Exception) error; - } - throw new Exception(error); + return; } } while (true); diff --git a/spring-boot/src/main/java/org/springframework/boot/LoggedExceptionHandler.java b/spring-boot/src/main/java/org/springframework/boot/LoggedExceptionHandler.java new file mode 100644 index 0000000000..82e5e8a5ac --- /dev/null +++ b/spring-boot/src/main/java/org/springframework/boot/LoggedExceptionHandler.java @@ -0,0 +1,82 @@ +/* + * Copyright 2012-2015 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.boot; + +import java.lang.Thread.UncaughtExceptionHandler; +import java.lang.reflect.InvocationTargetException; +import java.util.ArrayList; +import java.util.List; + +/** + * {@link UncaughtExceptionHandler} to suppress handling already logged exceptions. + * + * @author Phillip Webb + */ +class LoggedExceptionHandler implements UncaughtExceptionHandler { + + private static LoggedExceptionHandlerThreadLocal handler = new LoggedExceptionHandlerThreadLocal(); + + private final UncaughtExceptionHandler parent; + + private final List exceptions = new ArrayList(); + + LoggedExceptionHandler(UncaughtExceptionHandler parent) { + this.parent = parent; + } + + public void register(Throwable exception) { + this.exceptions.add(exception); + } + + @Override + public void uncaughtException(Thread thread, Throwable ex) { + if (!isRegistered(ex) && this.parent != null) { + this.parent.uncaughtException(thread, ex); + } + } + + private boolean isRegistered(Throwable ex) { + if (this.exceptions.contains(ex)) { + return true; + } + if (ex instanceof InvocationTargetException) { + return isRegistered(ex.getCause()); + } + return false; + } + + static LoggedExceptionHandler forCurrentThread() { + return handler.get(); + } + + /** + * Thread local used to attach and track handlers. + */ + private static class LoggedExceptionHandlerThreadLocal + extends ThreadLocal { + + @Override + protected LoggedExceptionHandler initialValue() { + LoggedExceptionHandler handler = new LoggedExceptionHandler( + Thread.currentThread().getUncaughtExceptionHandler()); + Thread.currentThread().setUncaughtExceptionHandler(handler); + return handler; + }; + + } + +} diff --git a/spring-boot/src/main/java/org/springframework/boot/SpringApplication.java b/spring-boot/src/main/java/org/springframework/boot/SpringApplication.java index bb6ed9ee42..53e52d3b41 100644 --- a/spring-boot/src/main/java/org/springframework/boot/SpringApplication.java +++ b/spring-boot/src/main/java/org/springframework/boot/SpringApplication.java @@ -301,17 +301,8 @@ public class SpringApplication { return context; } catch (Throwable ex) { - try { - listeners.finished(context, ex); - this.log.error("Application startup failed", ex); - } - finally { - if (context != null) { - context.close(); - } - } - ReflectionUtils.rethrowRuntimeException(ex); - return context; + handleRunFailure(context, listeners, ex); + throw new IllegalStateException(ex); } } @@ -816,6 +807,42 @@ public class SpringApplication { protected void afterRefresh(ConfigurableApplicationContext context, String[] args) { } + private void handleRunFailure(ConfigurableApplicationContext context, + SpringApplicationRunListeners listeners, Throwable exception) { + try { + try { + listeners.finished(context, exception); + } + finally { + if (context != null) { + context.close(); + } + } + } + catch (Exception ex) { + this.log.warn("Unable to close ApplicationContext", ex); + } + if (this.log.isErrorEnabled()) { + this.log.error("Application startup failed", exception); + registerLoggedException(exception); + } + ReflectionUtils.rethrowRuntimeException(exception); + } + + /** + * Register that the given exception has been logged. By default, if the running in + * the main thread, this method will suppress additional printing of the stacktrace. + * @param exception the exception that was logged + */ + protected void registerLoggedException(Throwable exception) { + Thread currentThread = Thread.currentThread(); + if (("main".equals(currentThread.getName()) + || "restartedMain".equals(currentThread.getName())) + && "main".equals(currentThread.getThreadGroup().getName())) { + LoggedExceptionHandler.forCurrentThread().register(exception); + } + } + /** * Set a specific main application class that will be used as a log source and to * obtain version information. By default the main application class will be deduced. diff --git a/spring-boot/src/test/java/org/springframework/boot/SpringApplicationTests.java b/spring-boot/src/test/java/org/springframework/boot/SpringApplicationTests.java index 6e1a1f2313..c92f660cfe 100644 --- a/spring-boot/src/test/java/org/springframework/boot/SpringApplicationTests.java +++ b/spring-boot/src/test/java/org/springframework/boot/SpringApplicationTests.java @@ -697,6 +697,25 @@ public class SpringApplicationTests { .next().getName()); } + @Test + public void failureResultsInSingleStackTrace() throws Exception { + ThreadGroup group = new ThreadGroup("main"); + Thread thread = new Thread(group, "main") { + @Override + public void run() { + SpringApplication application = new SpringApplication( + FailingConfig.class); + application.setWebEnvironment(false); + application.run(); + }; + }; + thread.start(); + thread.join(6000); + int occurrences = StringUtils.countOccurrencesOf(this.output.toString(), + "Caused by: java.lang.RuntimeException: ExpectedError"); + assertThat("Expected single stacktrace", occurrences, equalTo(1)); + } + private boolean hasPropertySource(ConfigurableEnvironment environment, Class propertySourceClass, String name) { for (PropertySource source : environment.getPropertySources()) { @@ -810,6 +829,16 @@ public class SpringApplicationTests { } + @Configuration + static class FailingConfig { + + @Bean + public Object fail() { + throw new RuntimeException("ExpectedError"); + } + + } + @Configuration static class CommandLineRunConfig {