Changeset 4618

Timestamp:

Oct 30, 2008, 11:41:41 AM (15 years ago)

Author:

Nicklas Nordborg

Message:

Fixes #1161: Improve error handling in the extension system

Location:

trunk

Files:

• src/clients/web/net/sf/basedb/clients/web/extensions/ExtensionsControl.java (modified) (2 diffs)
• src/clients/web/net/sf/basedb/clients/web/extensions/service/Services.java (modified) (1 diff)
• src/core/net/sf/basedb/util/error/ThrowableUtil.java (added)
• src/core/net/sf/basedb/util/extensions/ActionIterator.java (modified) (4 diffs)
• src/core/net/sf/basedb/util/extensions/ExtensionContext.java (modified) (4 diffs)
• src/core/net/sf/basedb/util/extensions/ExtensionPointContext.java (modified) (4 diffs)
• src/core/net/sf/basedb/util/extensions/ExtensionsInvoker.java (modified) (8 diffs)
• src/core/net/sf/basedb/util/extensions/InvokationContext.java (modified) (1 diff)
• src/core/net/sf/basedb/util/extensions/Registry.java (modified) (6 diffs)
• www/admin/extensions/details.jsp (modified) (7 diffs)
• www/admin/extensions/tree.jsp (modified) (2 diffs)
• www/admin/services/services.jsp (modified) (4 diffs)
• www/include/styles/main.css (modified) (1 diff)

trunk/src/clients/web/net/sf/basedb/clients/web/extensions/ExtensionsControl.java

@@ -444 +444 @@
 
   /**
+    Get information about the last error that happened
+    when rendering an extension point.
+    @param id The id of the extension point
+    @return The error, or null if no error information is available
+    @since 2.9
+  */
+  public Throwable getLastExtensionPointError(String id)
+  {
+    return registry.getLastExtensionPointError(id);
+  }
+
+  /**
     Enable/disable an extension point.
     @param extensionPointId The ID of the extension point to enable/disable
@@ -494 +506 @@
   {
     return registry.getExtension(id);
+  }
+  
+  /**
+    Get information about the last error that happened
+    when rendering an extension.
+    @param id The id of the extension
+    @return The error, or null if no error information is available
+    @since 2.9
+  */
+  public Throwable getLastExtensionError(String id)
+  {
+    return registry.getLastExtensionError(id);
   }
   

trunk/src/clients/web/net/sf/basedb/clients/web/extensions/service/Services.java

@@ -115 +115 @@
     ExtensionsFilter filter = extension == null ?
         settings : new SingleExtensionFilter(extension.getId());
-    return (ExtensionsInvoker<ServiceControllerAction>)ExtensionsControl.useExtensions(null, filter, EXTENSION_POINT_ID);
+    ExtensionsInvoker<?> invoker = ExtensionsControl.useExtensions(null, filter, EXTENSION_POINT_ID);
+    invoker.setClearErrors(true);
+    return (ExtensionsInvoker<ServiceControllerAction>)invoker;
   }
   

trunk/src/core/net/sf/basedb/util/extensions/ActionIterator.java

@@ -60 +60 @@
   // The current context
   private InvokationContext<A> currentContext;
+  private A currentAction;
   
   // An iterator over all usable extensions
@@ -129 +130 @@
     if (!hasNext) throw new NoSuchElementException();
     checkNext = true;
-    return actions[offset];
+    currentAction = actions[offset];
+    return currentAction;
   }
   /**
@@ -160 +162 @@
   /**
     Gets the renderer for the current action.
-  */
-  Renderer<? super A> getRenderer()
+    @since 2.9
+  */
+  public Renderer<? super A> getRenderer()
   {
     return currentContext.getRenderer();
   }
 
+  /**
+    Clear any error that has been registered for the current 
+    action.
+    @since 2.9
+    @see #setError(Throwable)
+  */
+  public void clearError()
+  {
+    currentContext.clearError();
+  }
+  
+  /**
+    Register an error for the current action. The error can be retreived
+    with {@link Registry#getLastExtensionError(String)}.
+    @param t The error to register
+    @since 2.9
+  */
+  public void setError(Throwable t)
+  {
+    currentContext.setError(currentAction, t);
+  }
+  
   /**
     Check that the actions array is a not-null, non-empty array
@@ -190 +215 @@
         Extension ext = getExtension();
         ClassCastException cc = new ClassCastException(actions[i] + " -> " + actionClass.getName());
+        currentContext.setError(actions[i], cc);
         log.error("Action '" + actions[i] + "' created by extension '" + ext.getId() + 
           "' is not of the expected class '" + actionClass.getName(), cc);

trunk/src/core/net/sf/basedb/util/extensions/ExtensionContext.java

@@ -22 +22 @@
 package net.sf.basedb.util.extensions;
 
+import net.sf.basedb.util.extensions.Registry.RegisteredExtension;
+
 /**
   Invokation context for an extension. Most
@@ -35 +37 @@
 {
   private final ExtensionPointContext<? super A> mainContext;
-  private final Extension<A> extension;
+  private final RegisteredExtension<A> extension;
   private Renderer<? super A> renderer;
   private boolean hasCreatedRenderer;
   
-  ExtensionContext(ExtensionPointContext<? super A> mainContext, Extension<A> extension)
+  ExtensionContext(ExtensionPointContext<? super A> mainContext, RegisteredExtension<A> extension)
   {
     this.mainContext = mainContext;
@@ -90 +92 @@
       catch (Throwable t)
       {
+        setError(null, t);
         log.error("Error preparing renderer factory for extension '" +
           extension.getId() + "': " + factory, t);
@@ -140 +143 @@
     catch (Throwable t)
     {
+      setError(null, t);
       log.error("Error creating actions for extension '" +
           extension.getId() + "' using factory: " + factory, t);      
     }
     return actions;
+  }
+  
+  @Override
+  protected void setError(A action, Throwable t)
+  {
+    extension.setError(action, t);
+  }
+  
+  @Override
+  protected void clearError()
+  {
+    extension.clearError();
   }
   // ----------------------------------------

trunk/src/core/net/sf/basedb/util/extensions/ExtensionPointContext.java

@@ -22 +22 @@
 package net.sf.basedb.util.extensions;
 
+import net.sf.basedb.util.extensions.Registry.RegisteredExtensionPoint;
+
 /**
   Invokation context for an extension point.
@@ -35 +37 @@
   private final ClientContext clientContext;
   private final Registry registry;
-  private final ExtensionPoint<? super A> extensionPoint;
+  private final RegisteredExtensionPoint<? super A> extensionPoint;
   private Renderer<? super A> renderer;
   private boolean hasCreatedRenderer;
   
   ExtensionPointContext(Registry registry, ClientContext clientContext, 
-      ExtensionPoint<? super A> extensionPoint)
+      RegisteredExtensionPoint<? super A> extensionPoint)
   {
     this.registry = registry;
@@ -99 +101 @@
       catch (Throwable t)
       {
+        setError(null, t);
         log.error("Error preparing renderer factory for extension point '" +
           extensionPoint.getId() + "': " + factory, t);
@@ -133 +136 @@
     return null;
   }
+  
+  @Override
+  protected void setError(A action, Throwable t)
+  {
+    extensionPoint.setError(action, t);
+  }
+  
+  @Override
+  protected void clearError()
+  {
+    extensionPoint.clearError();
+  }
   // ----------------------------------------
 

trunk/src/core/net/sf/basedb/util/extensions/ExtensionsInvoker.java

@@ -23 +23 @@
 
 import java.util.Collection;
-import java.util.Iterator;
 
 /**
@@ -58 +57 @@
     org.apache.log4j.LogManager.getLogger("net.sf.basedb.util.extensions.ExtensionsInvoker");
   
-  final Collection<InvokationContext<A>> contexts;
+  private final Collection<InvokationContext<A>> contexts;
+  private boolean clearErrors;
   
   /**
@@ -68 +68 @@
   }
   
+  /**
+    Set a flag to indicate if existing errors should be cleared 
+    before an action is rendered. If this setting is not enabled (default)
+    old errors are never cleared.
+    @param clearErrors TRUE to clear old errors, FALSE to keep them
+    @since 2.9
+  */
+  public void setClearErrors(boolean clearErrors)
+  {
+    this.clearErrors = clearErrors;
+  }
+
   /**
     Create an iterator that iterates over all {@link Action}:s created
@@ -95 +107 @@
       try
       {
+        if (clearErrors) it.clearError();
         action = it.next();
         renderer = it.getRenderer();
@@ -105 +118 @@
       catch (Throwable t)
       {
+        it.setError(t);
         log.error("Could not render action '" + action + "' with renderer '" + 
             renderer + "'", t);
@@ -121 +135 @@
   public void render(Renderer<A> renderer)
   {
-    Iterator<A> it = iterate();
+    ActionIterator<A> it = iterate();
     while (it.hasNext())
     {
@@ -127 +141 @@
       try
       {
+        if (clearErrors) it.clearError();
         action = it.next();
         renderer.render(action);
@@ -132 +147 @@
       catch (Throwable t)
       {
+        it.setError(t);
         log.error("Could not render action '" + action + "' with renderer '" + 
             renderer + "'", t);

trunk/src/core/net/sf/basedb/util/extensions/InvokationContext.java

@@ -111 +111 @@
   protected abstract A[] getActions();
   
+  /**
+    Register an error for the current extension or extension point. 
+    Registered errors can be retrieved by {@link Registry#getLastExtensionError(String)}
+    and {@link Registry#getLastExtensionPointError(String)}
+    @param action The action that caused the error
+    @param t The error
+    @since 2.9
+  */
+  protected abstract void setError(A action, Throwable t);
+  
+  /**
+    Clear the registered error for the current extension or extension point.
+    @since 2.9
+  */
+  protected abstract void clearError();
+  
 }

trunk/src/core/net/sf/basedb/util/extensions/Registry.java

@@ -309 +309 @@
   
   /**
+    Get information about the last error that happened
+    when rendering an extension point.
+    @param id The id of the extension point
+    @return The error, or null if no error information is available
+    @since 2.9
+  */
+  public Throwable getLastExtensionPointError(String id)
+  {
+    RegisteredExtensionPoint<?> rep = extensionPoints.get(id);
+    return rep == null ? null : rep.getLastError();
+  }
+  
+  /**
     Register an extension. If the extension is already registered,
     the description, position, action factory and renderer factory
@@ -393 +406 @@
     return extensions.get(id);
   }
+  
+  /**
+    Get information about the last error that happened
+    when rendering an extension.
+    @param id The id of the extension
+    @return The error, or null if no error information is available
+    @since 2.9
+  */
+  public Throwable getLastExtensionError(String id)
+  {
+    RegisteredExtension<?> ext = extensions.get(id);
+    return ext == null ? null : ext.getLastError();
+  }
+
   
   /**
@@ -555 +582 @@
     private boolean allowRendererOverride;
     
+    private Throwable lastError;
+    private A lastErrorAction;
+    
     /**
       Create a new registered extension point by copying the
@@ -647 +677 @@
     {
       return this.extensions.values();
-    } 
+    }
+    
+    synchronized void setError(A action, Throwable t)
+    {
+      this.lastErrorAction = action;
+      this.lastError = t;
+    }
+    
+    synchronized void clearError()
+    {
+      this.lastError = null;
+      this.lastErrorAction = null;
+    }
+    
+    Throwable getLastError()
+    {
+      return lastError;
+    }
   }
     
@@ -665 +712 @@
     private RendererFactory<? super A> rendererFactory;
 
+    private Throwable lastError;
+    private A lastErrorAction;
+    
     /**
       Create a new registered extension by copying the
@@ -756 +806 @@
       return rep;
     }
+
+    synchronized void setError(A action, Throwable t)
+    {
+      this.lastErrorAction = action;
+      this.lastError = t;
+    }
+    
+    synchronized void clearError()
+    {
+      this.lastError = null;
+      this.lastErrorAction = null;
+    }
+    
+    Throwable getLastError()
+    {
+      return lastError;
+    }
   }   
     

trunk/www/admin/extensions/details.jsp

@@ -33 +33 @@
   import="net.sf.basedb.util.Values"
   import="net.sf.basedb.core.ItemNotFoundException"
+  import="net.sf.basedb.util.error.ThrowableUtil"
   import="net.sf.basedb.util.extensions.ExtensionPoint"
   import="net.sf.basedb.util.extensions.Extension"
@@ -169 +170 @@
     Main.openPopup('index.jsp?ID=<%=ID%>&cmd=ScanResults', 'ScanResults', 600, 480);
   }
+
+  function toggleStacktrace(evt, id)
+  {
+    Main.showHide('stacktrace.' + id);
+  }
+  
   </script>
   </base:head>
@@ -179 +186 @@
     {
       boolean isEnabled = ec.isEnabled(ext);
-      boolean hasError = extFile != null && extFile.hasError();
-      boolean allow = writePermission && !hasError;
+      boolean hasRegistrationError = extFile != null && extFile.hasError();
+      boolean allow = writePermission && !hasRegistrationError;
       %>
       <tbl:button 
@@ -200 +207 @@
     {
       boolean isEnabled = ec.isEnabled(ep);
-      boolean hasError = epFile != null && epFile.hasError();
-      boolean allow = writePermission && !hasError;
+      boolean hasRegistrationError = epFile != null && epFile.hasError();
+      boolean allow = writePermission && !hasRegistrationError;
       %>
       <tbl:button 
@@ -220 +227 @@
     else if (file != null)
     {
-      boolean hasError = file.hasError();
-      boolean allow = writePermission && !hasError;
+      boolean hasRegistrationError = file.hasError();
+      boolean allow = writePermission && !hasRegistrationError;
       %>
       <tbl:button 
@@ -274 +281 @@
       About about = ext.getAbout();
       if (about == null) about = new AboutBean();
-      %>
-      </a>
+      Throwable error = ec.getLastExtensionError(ext.getId());
+      if (error != null)
+      {
+        %>
+          <div class="error">
+            <base:icon image="error.gif" 
+              onclick="<%="toggleStacktrace(event, '" + ext.getId() + "')"%>" 
+              tooltip="Toggle display of detailed stacktrace"
+              style="float: left;"
+              tooltip="Error - click to show full stack trace"
+            /><%=error.getMessage() %>
+            <div id="stacktrace.<%=ext.getId()%>"
+              class="stacktrace"
+              style="display: none;"
+              ><%=ThrowableUtil.stackTraceToString(error)%></div>
+          </div>
+          <%
+      }
+      %>
         <table class="form" cellspacing="0">
         <tr>
@@ -340 +364 @@
         <h4>Extension point</h4>
         <%
+      }
+      Throwable error = ec.getLastExtensionPointError(ep.getId());
+      if (error != null)
+      {
+        %>
+          <div class="error">
+            <base:icon image="error.gif" 
+              onclick="<%="toggleStacktrace(event, '" + ep.getId() + "')"%>" 
+              tooltip="Toggle display of detailed stacktrace"
+              style="float: left;"
+              tooltip="Error - click to show full stack trace"
+            /><%=error.getMessage() %>
+            <div id="stacktrace.<%=ep.getId()%>"
+              class="stacktrace"
+              style="display: none;"
+              ><%=ThrowableUtil.stackTraceToString(error)%></div>
+          </div>
+          <%
       }
       %>

trunk/www/admin/extensions/tree.jsp

@@ -49 +49 @@
   String icon = ec.isEnabled(ep) ? "ExtensionPoint" : "ExtensionPointDisabled";
   ExtensionsFile f = ec.getFileByExtensionId(id);
-  if (f == null || f.hasError()) icon = "ExtensionPointError";
+  if (f == null || f.hasError() || ec.getLastExtensionPointError(id) != null) 
+  {
+    icon = "ExtensionPointError";
+  }
   String joust = "ep = JoustMenu.addChildItem(" + parentNode +", '" + icon + "', " + 
       "'" + HTML.javaScriptEncode(name) + "', 'extensionPointOnClick(\"" + id + "\")', " + 
@@ -62 +65 @@
   String icon = ec.isEnabled(ext) ? "Extension" : "ExtensionDisabled";
   ExtensionsFile f = ec.getFileByExtensionId(id);
-  if (f == null || f.hasError()) icon = "ExtensionError";
+  if (f == null || f.hasError() || ec.getLastExtensionError(id) != null)
+  {
+    icon = "ExtensionError";
+  }
   String joust = "ext = JoustMenu.addChildItem(" + parentNode + ", '" + icon + "', " + 
       "'" + HTML.javaScriptEncode(name) + "', 'extensionOnClick(\"" + id + "\")', " + 

trunk/www/admin/services/services.jsp

@@ -4 +4 @@
   import="net.sf.basedb.core.DbControl"
   import="net.sf.basedb.core.Permission"
+  import="net.sf.basedb.util.error.ThrowableUtil"
   import="net.sf.basedb.util.extensions.ActionIterator"
   import="net.sf.basedb.util.extensions.Extension"
   import="net.sf.basedb.util.extensions.ExtensionsFilter"
   import="net.sf.basedb.util.extensions.ExtensionsInvoker"
+  import="net.sf.basedb.util.extensions.Registry"
   import="net.sf.basedb.clients.web.Base"
   import="net.sf.basedb.clients.web.extensions.ExtensionsControl"
@@ -46 +48 @@
   {
     location.href = 'index.jsp?ID=<%=ID%>&cmd=Start&extensionId=' + extensionId;
+  }
+  function toggleStacktrace(evt, extensionId)
+  {
+    Main.showHide('stacktrace.' + extensionId);
   }
   </script>
@@ -86 +92 @@
           last = ext;
           boolean isRunning = action.isRunning();
+          Throwable error = ec.getLastExtensionError(ext.getId());
+          boolean hasError = error != null;
           %>
           <tbl:row>
@@ -99 +107 @@
               </tbl:cell>
             <tbl:cell column="service"><%=action.toString()%></tbl:cell>
-            <tbl:cell column="status"><%=isRunning ? "Running" : "Stopped"%></tbl:cell>
+            <tbl:cell column="status">
+              <%
+              if (hasError)
+              {
+                %>
+                <div class="error">
+                <base:icon image="error.gif" 
+                    onclick="<%="toggleStacktrace(event, '" + ext.getId() + "')"%>" 
+                    tooltip="Toggle display of detailed stacktrace"
+                    style="float: left;"
+                    tooltip="Error - click to show full stack trace"
+                /><%=error.getMessage() %>
+                <div id="stacktrace.<%=ext.getId()%>"
+                    class="stacktrace"
+                    style="display: none;"
+                  ><%=ThrowableUtil.stackTraceToString(error)%></div>
+                </div>
+                <%
+              }
+              else
+              {
+                %>
+                <%=isRunning ? "Running" : "Stopped"%>
+                <%
+              }
+              %>
+            </tbl:cell>
             <tbl:cell column="actions">
               <%

trunk/www/include/styles/main.css

@@ -228 +228 @@
 }
 
+.error .stacktrace {
+  white-space: pre;
+  font-family: monospace;
+  text-align: left; 
+  font-size: smaller; 
+  background: #ffffd8; 
+  color: #000000; 
+  padding: 2px;
+}
+
 .helpmessage {
   background: #E0E0E0;