Nicer error pages

jenkinsci · Aug 28, 2023 · 690edb0 · 690edb0
1 parent 97c46e1
commit 690edb0
Show file tree

Hide file tree

Showing 6 changed files with 112 additions and 20 deletions.
diff --git a/plugin/src/main/java/io/jenkins/plugins/casc/BaseConfigurator.java b/plugin/src/main/java/io/jenkins/plugins/casc/BaseConfigurator.java
@@ -377,14 +377,16 @@ protected void configure(Mapping config, T instance, boolean dryrun, Configurati
     protected final void handleUnknown(Mapping config, ConfigurationContext context) throws ConfiguratorException {
         if (!config.isEmpty()) {
             final String invalid = StringUtils.join(config.keySet(), ',');
+            List<String> validAttributes = getAttributes().stream().map(Attribute::getName)
+                .collect(Collectors.toList());
             final String message = "Invalid configuration elements for type " + getTarget() + " : " + invalid + ".\n"
                     + "Available attributes : "
                     + StringUtils.join(
-                            getAttributes().stream().map(Attribute::getName).collect(Collectors.toList()), ", ");
+                validAttributes, ", ");
             context.warning(config, message);
             switch (context.getUnknown()) {
                 case reject:
-                    throw new ConfiguratorException(message);
+                    throw new ConfiguratorException(this, message, invalid, validAttributes, null);
 
                 case warn:
                     LOGGER.warning(message);

diff --git a/plugin/src/main/java/io/jenkins/plugins/casc/ConfigurationAsCode.java b/plugin/src/main/java/io/jenkins/plugins/casc/ConfigurationAsCode.java
@@ -169,7 +169,28 @@ public void doReload(StaplerRequest request, StaplerResponse response) throws Ex
             response.sendError(HttpServletResponse.SC_FORBIDDEN);
             return;
         }
-        configure();
+
+        try {
+            configure();
+        } catch (ConfiguratorException e) {
+            LOGGER.log(Level.SEVERE, "Failed to reload configuration", e);
+
+            Throwable throwableCause = e.getCause();
+            if (throwableCause instanceof ConfiguratorException) {
+                ConfiguratorException cause = (ConfiguratorException) throwableCause;
+
+                Configurator<?> configurator = cause.getConfigurator();
+                if (configurator != null) {
+                    request.setAttribute("target", configurator.getName());
+                }
+                request.setAttribute("invalidAttribute", cause.getInvalidAttribute());
+                request.setAttribute("validAttributes", cause.getValidAttributes());
+                request.getView(this, "error.jelly").forward(request, response);
+                return;
+            } else {
+                throw e;
+            }
+        }
         response.sendRedirect("");
     }
 
@@ -303,7 +324,11 @@ private List<YamlSource> getConfigFromSources(List<String> newSources) throws Co
     @Initializer(after = InitMilestone.SYSTEM_CONFIG_LOADED, before = InitMilestone.SYSTEM_CONFIG_ADAPTED)
     public static void init() throws Exception {
         detectVaultPluginMissing();
-        get().configure();
+        try {
+            get().configure();
+        } catch (ConfiguratorException e) {
+            throw new ConfigurationAsCodeBootFailure(e);
+        }
     }
 
     /**
@@ -727,18 +752,9 @@ private static void invokeWith(Mapping entries, ConfiguratorOperation function)
                 if (!entry.getKey().equalsIgnoreCase(configurator.getName())) {
                     continue;
                 }
-                try {
-                    function.apply(configurator, entry.getValue());
-                    it.remove();
-                    break;
-                } catch (ConfiguratorException e) {
-                    throw new ConfiguratorException(
-                            configurator,
-                            format(
-                                    "error configuring '%s' with %s configurator",
-                                    entry.getKey(), configurator.getClass()),
-                            e);
-                }
+                function.apply(configurator, entry.getValue());
+                it.remove();
+                break;
             }
         }
 

diff --git a/plugin/src/main/java/io/jenkins/plugins/casc/ConfigurationAsCodeBootFailure.java b/plugin/src/main/java/io/jenkins/plugins/casc/ConfigurationAsCodeBootFailure.java
@@ -0,0 +1,10 @@
+package io.jenkins.plugins.casc;
+
+import hudson.util.BootFailure;
+
+public class ConfigurationAsCodeBootFailure extends BootFailure {
+
+    public ConfigurationAsCodeBootFailure(Throwable cause) {
+        super(cause);
+    }
+}
diff --git a/plugin/src/main/java/io/jenkins/plugins/casc/ConfiguratorException.java b/plugin/src/main/java/io/jenkins/plugins/casc/ConfiguratorException.java
@@ -24,6 +24,8 @@
 import edu.umd.cs.findbugs.annotations.CheckForNull;
 import io.jenkins.plugins.casc.model.CNode;
 import java.io.IOException;
+import java.util.Collections;
+import java.util.List;
 
 /**
  * Exception type for {@link Configurator} issues.
@@ -37,33 +39,59 @@ public class ConfiguratorException extends IOException {
     @CheckForNull
     private final Configurator configurator;
 
+    private final List<String> validAttributes;
+    @CheckForNull
+    private final String invalidAttribute;
+
+    public ConfiguratorException(
+        @CheckForNull Configurator configurator, @CheckForNull String message,
+        String invalidAttribute,
+        List<String> validAttributes,
+        @CheckForNull Throwable cause) {
+        super(message, cause);
+        this.configurator = configurator;
+
+        this.invalidAttribute = invalidAttribute;
+        this.validAttributes = validAttributes;
+    }
+
     public ConfiguratorException(
             @CheckForNull Configurator configurator, @CheckForNull String message, @CheckForNull Throwable cause) {
         super(message, cause);
         this.configurator = configurator;
+        this.invalidAttribute = null;
+        this.validAttributes = Collections.emptyList();
     }
 
     public ConfiguratorException(@CheckForNull String message, @CheckForNull Throwable cause) {
-        this(null, message, cause);
+        this(null, message, null, Collections.emptyList(), cause);
     }
 
     public ConfiguratorException(@CheckForNull Configurator configurator, @CheckForNull String message) {
-        this(configurator, message, null);
+        this(configurator, message, null, Collections.emptyList(), null);
     }
 
     public ConfiguratorException(@CheckForNull String message) {
-        this(null, message, null);
+        this(null, message, null, Collections.emptyList(), null);
     }
 
     public ConfiguratorException(@CheckForNull Throwable cause) {
-        this(null, null, cause);
+        this(null, null, null, Collections.emptyList(), cause);
     }
 
     @CheckForNull
     public Configurator getConfigurator() {
         return configurator;
     }
 
+    public List<String> getValidAttributes() {
+        return validAttributes;
+    }
+
+    public String getInvalidAttribute() {
+        return invalidAttribute;
+    }
+
     @Override
     public String getMessage() {
         if (configurator != null) {

diff --git a/plugin/src/main/resources/io/jenkins/plugins/casc/ConfigurationAsCode/error.jelly b/plugin/src/main/resources/io/jenkins/plugins/casc/ConfigurationAsCode/error.jelly
@@ -0,0 +1,18 @@
+<?jelly escape-by-default='true'?>
+<j:jelly xmlns:j="jelly:core" xmlns:l="/lib/layout">
+  <l:layout type="one-column" title="${%Error loading configuration}">
+    <l:main-panel>
+     <l:app-bar title="${%Error loading configuration}" />
+
+      <p>Invalid configuration elements for configurator with name: <pre><code>${target}</code></pre></p>
+      <p>Attribute was: <pre><code>${invalidAttribute}</code></pre></p>
+      <p>Valid attributes are:</p>
+
+      <ul>
+        <j:forEach var="attribute" items="${validAttributes}">
+          <li>${attribute}</li>
+        </j:forEach>
+      </ul>
+    </l:main-panel>
+  </l:layout>
+</j:jelly>
diff --git a/plugin/src/main/resources/io/jenkins/plugins/casc/ConfigurationAsCodeBootFailure/index.jelly b/plugin/src/main/resources/io/jenkins/plugins/casc/ConfigurationAsCodeBootFailure/index.jelly
@@ -0,0 +1,18 @@
+<?jelly escape-by-default='true'?>
+<j:jelly xmlns:j="jelly:core" xmlns:l="/lib/layout">
+  <l:layout type="one-column" title="${%Error loading configuration}">
+    <l:main-panel>
+      <l:app-bar title="${%Error loading configuration}" />
+
+      <p>Invalid configuration elements for configurator with name: <pre><code>${it.cause.configurator.name}</code></pre></p>
+      <p>Attribute was: <pre><code>${it.cause.invalidAttribute}</code></pre></p>
+      <p>Valid attributes are:</p>
+
+      <ul>
+        <j:forEach var="attribute" items="${it.cause.validAttributes}">
+          <li>${attribute}</li>
+        </j:forEach>
+      </ul>
+    </l:main-panel>
+  </l:layout>
+</j:jelly>