Merge pull request #14692 from stuartwdouglas/fix-config-on-error

Add mini config editor to error page

core/deployment/src/main/java/io/quarkus/deployment/configuration/BuildTimeConfigurationReader.java

@@ -14,6 +14,7 @@
 import java.lang.reflect.ParameterizedType;
 import java.lang.reflect.Type;
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
@@ -51,6 +52,7 @@
 import io.quarkus.runtime.annotations.ConfigPhase;
 import io.quarkus.runtime.annotations.ConfigRoot;
 import io.quarkus.runtime.configuration.ConfigUtils;
+import io.quarkus.runtime.configuration.ConfigurationException;
 import io.quarkus.runtime.configuration.HyphenateEnumConverter;
 import io.quarkus.runtime.configuration.NameIterator;
 import io.smallrye.config.Converters;
@@ -626,11 +628,15 @@ private void readConfigGroup(ClassDefinition definition, Object instance, final
         private void readConfigValue(String fullName, ClassDefinition.ItemMember member, Object instance) {
             Field field = member.getField();
             Converter<?> converter = getConverter(config, field, ConverterType.of(field));
-            Object val = config.getValue(fullName, converter);
+
             try {
+                Object val = config.getValue(fullName, converter);
                 field.set(instance, val);
             } catch (IllegalAccessException e) {
                 throw toError(e);
+            } catch (Exception e) {
+                throw new ConfigurationException(e.getMessage(), e, Collections.singleton(fullName));
+
             }
         }
 

core/deployment/src/main/java/io/quarkus/deployment/configuration/RunTimeConfigurationGenerator.java

@@ -119,6 +119,8 @@ public final class RunTimeConfigurationGenerator {
             void.class, String.class, IllegalArgumentException.class);
     static final MethodDescriptor CD_IS_ERROR = MethodDescriptor.ofMethod(ConfigDiagnostic.class, "isError",
             boolean.class);
+    static final MethodDescriptor CD_GET_ERROR_KEYS = MethodDescriptor.ofMethod(ConfigDiagnostic.class, "getErrorKeys",
+            Set.class);
     static final MethodDescriptor CD_MISSING_VALUE = MethodDescriptor.ofMethod(ConfigDiagnostic.class, "missingValue",
             void.class, String.class, NoSuchElementException.class);
     static final MethodDescriptor CD_RESET_ERROR = MethodDescriptor.ofMethod(ConfigDiagnostic.class, "resetError", void.class);
@@ -830,6 +832,7 @@ public void run() {
             ResultHandle niceErrorMessage = isError
                     .invokeStaticMethod(
                             MethodDescriptor.ofMethod(ConfigDiagnostic.class, "getNiceErrorMessage", String.class));
+            ResultHandle errorKeys = isError.invokeStaticMethod(CD_GET_ERROR_KEYS);
             isError.invokeStaticMethod(CD_RESET_ERROR);
 
             // throw the proper exception
@@ -839,7 +842,8 @@ public void run() {
             isError.invokeVirtualMethod(SB_APPEND_STRING, finalErrorMessageBuilder, niceErrorMessage);
             final ResultHandle finalErrorMessage = isError.invokeVirtualMethod(OBJ_TO_STRING, finalErrorMessageBuilder);
             final ResultHandle configurationException = isError
-                    .newInstance(MethodDescriptor.ofConstructor(ConfigurationException.class, String.class), finalErrorMessage);
+                    .newInstance(MethodDescriptor.ofConstructor(ConfigurationException.class, String.class, Set.class),
+                            finalErrorMessage, errorKeys);
             final ResultHandle emptyStackTraceElement = isError.newArray(StackTraceElement.class, 0);
             // empty out the stack trace in order to not make the configuration errors more visible (the stack trace contains generated classes anyway that don't provide any value)
             isError.invokeVirtualMethod(

core/deployment/src/main/java/io/quarkus/deployment/dev/IsolatedDevModeMain.java

@@ -54,6 +54,7 @@
 import io.quarkus.deployment.steps.ClassTransformingBuildStep;
 import io.quarkus.deployment.util.FSWatchUtil;
 import io.quarkus.dev.console.DevConsoleManager;
+import io.quarkus.dev.spi.DeploymentFailedStartHandler;
 import io.quarkus.dev.spi.DevModeType;
 import io.quarkus.dev.spi.HotReplacementSetup;
 import io.quarkus.runner.bootstrap.AugmentActionImpl;
@@ -98,7 +99,8 @@ private synchronized void firstStart(QuarkusClassLoader deploymentClassLoader, L
                             @Override
                             public void accept(Integer integer) {
                                 if (restarting || ApplicationLifecycleManager.isVmShuttingDown()
-                                        || context.isAbortOnFailedStart()) {
+                                        || context.isAbortOnFailedStart() ||
+                                        context.isTest()) {
                                     return;
                                 }
                                 if (consoleContext == null) {
@@ -271,6 +273,21 @@ public byte[] apply(String s, byte[] bytes) {
                 service.setupHotDeployment(processor);
                 processor.addHotReplacementSetup(service);
             }
+            for (DeploymentFailedStartHandler service : ServiceLoader.load(DeploymentFailedStartHandler.class,
+                    curatedApplication.getAugmentClassLoader())) {
+                processor.addDeploymentFailedStartHandler(new Runnable() {
+                    @Override
+                    public void run() {
+                        ClassLoader old = Thread.currentThread().getContextClassLoader();
+                        try {
+                            Thread.currentThread().setContextClassLoader(curatedApplication.getAugmentClassLoader());
+                            service.handleFailedInitialStart();
+                        } finally {
+                            Thread.currentThread().setContextClassLoader(old);
+                        }
+                    }
+                });
+            }
             DevConsoleManager.setQuarkusBootstrap(curatedApplication.getQuarkusBootstrap());
             DevConsoleManager.setHotReplacementContext(processor);
             return processor;

core/deployment/src/main/java/io/quarkus/deployment/dev/IsolatedRemoteDevModeMain.java

@@ -40,6 +40,7 @@
 import io.quarkus.deployment.pkg.PackageConfig;
 import io.quarkus.deployment.pkg.steps.JarResultBuildStep;
 import io.quarkus.deployment.steps.ClassTransformingBuildStep;
+import io.quarkus.dev.spi.DeploymentFailedStartHandler;
 import io.quarkus.dev.spi.DevModeType;
 import io.quarkus.dev.spi.HotReplacementSetup;
 import io.quarkus.dev.spi.RemoteDevState;
@@ -143,6 +144,21 @@ public byte[] apply(String s, byte[] bytes) {
                 service.setupHotDeployment(processor);
                 processor.addHotReplacementSetup(service);
             }
+            for (DeploymentFailedStartHandler service : ServiceLoader.load(DeploymentFailedStartHandler.class,
+                    curatedApplication.getAugmentClassLoader())) {
+                processor.addDeploymentFailedStartHandler(new Runnable() {
+                    @Override
+                    public void run() {
+                        ClassLoader old = Thread.currentThread().getContextClassLoader();
+                        try {
+                            Thread.currentThread().setContextClassLoader(curatedApplication.getAugmentClassLoader());
+                            service.handleFailedInitialStart();
+                        } finally {
+                            Thread.currentThread().setContextClassLoader(old);
+                        }
+                    }
+                });
+            }
             return processor;
         }
         return null;

core/deployment/src/main/java/io/quarkus/deployment/dev/RuntimeUpdatesProcessor.java

@@ -99,6 +99,7 @@ public class RuntimeUpdatesProcessor implements HotReplacementContext, Closeable
     private final List<Runnable> preScanSteps = new CopyOnWriteArrayList<>();
     private final List<Consumer<Set<String>>> noRestartChangesConsumers = new CopyOnWriteArrayList<>();
     private final List<HotReplacementSetup> hotReplacementSetup = new ArrayList<>();
+    private final List<Runnable> deploymentFailedStartHandlers = new ArrayList<>();
     private final BiConsumer<Set<String>, ClassScanResult> restartCallback;
     private final BiConsumer<DevModeContext.ModuleInfo, String> copyResourceNotification;
     private final BiFunction<String, byte[], byte[]> classTransformers;
@@ -261,7 +262,16 @@ public void run() {
                             }, 1, 1000);
                         }
                     }
-                    periodicTestCompile();
+                    //this can't be called directly because of the deadlock risk
+                    //this can happen on a hot reload, if you have changed the config to make testing 'enabled'
+                    //the thread doing the reload already holds the lock, so a deadlock would result
+                    testClassChangeTimer.schedule(new TimerTask() {
+                        @Override
+                        public void run() {
+                            periodicTestCompile();
+                        }
+                    }, 0);
+
                 } else {
                     testClassChangeTimer = new Timer("Test Compile Timer", true);
                     testClassChangeTimer.schedule(new TimerTask() {
@@ -1047,10 +1057,17 @@ public void addHotReplacementSetup(HotReplacementSetup service) {
         hotReplacementSetup.add(service);
     }
 
+    public void addDeploymentFailedStartHandler(Runnable service) {
+        deploymentFailedStartHandlers.add(service);
+    }
+
     public void startupFailed() {
         for (HotReplacementSetup i : hotReplacementSetup) {
             i.handleFailedInitialStart();
         }
+        for (Runnable i : deploymentFailedStartHandlers) {
+            i.run();
+        }
         //if startup failed we always do a class loader based restart
         lastStartIndex = null;
     }

core/devmode-spi/src/main/java/io/quarkus/dev/config/ConfigurationProblem.java

@@ -0,0 +1,13 @@
+package io.quarkus.dev.config;
+
+import java.util.Set;
+
+/**
+ * Interface that can be implemented by exceptions to allow for config issues to be easily fixed in dev mode.
+ *
+ */
+public interface ConfigurationProblem {
+
+    Set<String> getConfigKeys();
+
+}

core/devmode-spi/src/main/java/io/quarkus/dev/config/CurrentConfig.java

@@ -0,0 +1,59 @@
+package io.quarkus.dev.config;
+
+import java.util.Collections;
+import java.util.List;
+import java.util.Map;
+import java.util.function.Consumer;
+
+public class CurrentConfig implements Comparable<CurrentConfig> {
+
+    public static volatile List<CurrentConfig> CURRENT = Collections.emptyList();
+    public static volatile Consumer<Map<String, String>> EDITOR;
+
+    private final String propertyName;
+    private final String description;
+    private final String defaultValue;
+    private final String currentValue;
+    private final String appPropertiesValue;
+
+    public CurrentConfig(String propertyName, String description, String defaultValue, String currentValue,
+            String appPropertiesValue) {
+        this.propertyName = propertyName;
+        this.description = description;
+        this.defaultValue = defaultValue;
+        this.currentValue = currentValue;
+        this.appPropertiesValue = appPropertiesValue;
+    }
+
+    public String getPropertyName() {
+        return propertyName;
+    }
+
+    public String getDescription() {
+        return description;
+    }
+
+    public String getDefaultValue() {
+        return defaultValue;
+    }
+
+    public String getCurrentValue() {
+        return currentValue;
+    }
+
+    public String getAppPropertiesValue() {
+        return appPropertiesValue;
+    }
+
+    @Override
+    public int compareTo(CurrentConfig o) {
+        if (appPropertiesValue == null && o.appPropertiesValue != null) {
+            return 1;
+        }
+        if (appPropertiesValue != null && o.appPropertiesValue == null) {
+            return -1;
+        }
+
+        return propertyName.compareTo(o.propertyName);
+    }
+}

core/devmode-spi/src/main/java/io/quarkus/dev/spi/DeploymentFailedStartHandler.java

@@ -0,0 +1,11 @@
+package io.quarkus.dev.spi;
+
+public interface DeploymentFailedStartHandler {
+
+    /**
+     * This method is called if the app fails to start the first time. This allows for hot deployment
+     * providers to still start, and provide a way for the user to recover their app
+     */
+    void handleFailedInitialStart();
+
+}

core/runtime/src/main/java/io/quarkus/runtime/TemplateHtmlBuilder.java

@@ -1,5 +1,9 @@
 package io.quarkus.runtime;
 
+import java.util.Collections;
+import java.util.List;
+
+import io.quarkus.dev.config.CurrentConfig;
 import io.quarkus.runtime.util.ExceptionUtil;
 
 public class TemplateHtmlBuilder {
@@ -186,12 +190,50 @@ public class TemplateHtmlBuilder {
             "    color: #555;\n" +
             "}\n";
 
+    private static final String CONFIG_EDITOR_HEAD = "<h3>The following incorrect config values were detected:</h3>" +
+            "<form method=\"post\" enctype=\"application/x-www-form-urlencoded\"  action=\"/io.quarkus.vertx-http.devmode.config.fix\">"
+            +
+            "<input type=\"hidden\" name=\"redirect\" value=\"%s\"/>\n" +
+            "<table class=\"table table-striped\" cellspacing=\"20\">\n" +
+            "    <thead class=\"thead-dark\">\n" +
+            "    <tr>\n" +
+            "        <th scope=\"col\">Config Key</th>\n" +
+            "        <th scope=\"col\">Value</th>\n" +
+            "    </tr>\n" +
+            "    </thead>\n" +
+            "    <tbody>\n";
+
+    private static final String CONFIG_EDITOR_ROW = "    <tr style=\"padding:12px\">\n" +
+            "            <td>\n" +
+            "                %s\n" +
+            "            </td>\n" +
+            "            <td>\n" +
+            "                <input type=\"text\" name=\"key.%s\" value=\"%s\"/>\n" +
+            "            </td>\n" +
+            "    </tr>\n";
+
+    private static final String CONFIG_EDITOR_TAIL = "    </tbody>\n" +
+            "</table>" +
+            "<input type=\"submit\" value=\"Update\" >" +
+            "</form>";
     private StringBuilder result;
 
     public TemplateHtmlBuilder(String title, String subTitle, String details) {
+        this(title, subTitle, details, null, Collections.emptyList());
+    }
+
+    public TemplateHtmlBuilder(String title, String subTitle, String details, String redirect, List<CurrentConfig> config) {
         result = new StringBuilder(String.format(HTML_TEMPLATE_START, escapeHtml(title),
                 subTitle == null || subTitle.isEmpty() ? "" : " - " + escapeHtml(subTitle), ERROR_CSS));
         result.append(String.format(HEADER_TEMPLATE, escapeHtml(title), escapeHtml(details)));
+        if (!config.isEmpty()) {
+            result.append(String.format(CONFIG_EDITOR_HEAD, redirect));
+            for (CurrentConfig i : config) {
+                result.append(String.format(CONFIG_EDITOR_ROW, escapeHtml(i.getPropertyName()), escapeHtml(i.getPropertyName()),
+                        escapeHtml(i.getCurrentValue())));
+            }
+            result.append(CONFIG_EDITOR_TAIL);
+        }
     }
 
     public TemplateHtmlBuilder stack(final Throwable throwable) {

core/runtime/src/main/java/io/quarkus/runtime/configuration/ConfigDiagnostic.java

@@ -1,9 +1,12 @@
 package io.quarkus.runtime.configuration;
 
 import java.util.ArrayList;
+import java.util.HashSet;
 import java.util.List;
 import java.util.NoSuchElementException;
+import java.util.Set;
 import java.util.concurrent.CopyOnWriteArrayList;
+import java.util.concurrent.CopyOnWriteArraySet;
 
 import org.eclipse.microprofile.config.Config;
 import org.eclipse.microprofile.config.ConfigProvider;
@@ -22,6 +25,7 @@ public final class ConfigDiagnostic {
 
     @RecomputeFieldValue(kind = RecomputeFieldValue.Kind.Reset)
     private static final List<String> errorsMessages = new CopyOnWriteArrayList<>();
+    private static final Set<String> errorKeys = new CopyOnWriteArraySet<>();
 
     private ConfigDiagnostic() {
     }
@@ -31,18 +35,21 @@ public static void invalidValue(String name, IllegalArgumentException ex) {
         final String loggedMessage = message != null ? message
                 : String.format("An invalid value was given for configuration key \"%s\"", name);
         errorsMessages.add(loggedMessage);
+        errorKeys.add(name);
     }
 
     public static void missingValue(String name, NoSuchElementException ex) {
         final String message = ex.getMessage();
         final String loggedMessage = message != null ? message
                 : String.format("Configuration key \"%s\" is required, but its value is empty/missing", name);
         errorsMessages.add(loggedMessage);
+        errorKeys.add(name);
     }
 
     public static void duplicate(String name) {
         final String loggedMessage = String.format("Configuration key \"%s\" was specified more than once", name);
         errorsMessages.add(loggedMessage);
+        errorKeys.add(name);
     }
 
     public static void deprecated(String name) {
@@ -127,6 +134,7 @@ public static boolean isError() {
      * Reset the config error status (for e.g. testing).
      */
     public static void resetError() {
+        errorKeys.clear();
         errorsMessages.clear();
     }
 
@@ -139,4 +147,8 @@ public static String getNiceErrorMessage() {
         }
         return b.toString();
     }
+
+    public static Set<String> getErrorKeys() {
+        return new HashSet<>(errorKeys);
+    }
 }