Skip to content

Commit

Permalink
spacemanager: Log unexpected exceptions
Browse files Browse the repository at this point in the history
Undeclared exceptions thrown by shell commands are usually logged as bugs
and rethrown as CommandPanicExceptions, however that doesn't apply to
DelayedCommands. This patch resolve the problem.

DelayedCommands are mostly used by space manager.

Target: trunk
Require-notes: no
Require-book: no
Request: 2.11
Request: 2.10
Acked-by: Albert Rossi <arossi@fnal.gov>
Patch: https://rb.dcache.org/r/7844/
(cherry picked from commit f47beea)

Conflicts:
	modules/cells/src/main/java/dmg/util/command/DelayedCommand.java

(cherry picked from commit 717520d)
  • Loading branch information
gbehrmann committed Feb 23, 2015
1 parent 8d37d0a commit 806e8c0
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 38 deletions.
17 changes: 17 additions & 0 deletions modules/cells/src/main/java/dmg/util/command/DelayedCommand.java
@@ -1,11 +1,18 @@
package dmg.util.command;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import java.io.Serializable;
import java.lang.reflect.Method;
import java.util.concurrent.Callable;
import java.util.concurrent.Executor;

import dmg.cells.nucleus.DelayedReply;
import dmg.cells.nucleus.Reply;
import dmg.util.CommandPanicException;

import org.dcache.util.ReflectionUtils;

/**
* Abstract base class for annotated commands for executing the command
Expand All @@ -25,6 +32,7 @@ public void execute(Runnable command)
new Thread(command).start();
}
};
private static final Logger LOGGER = LoggerFactory.getLogger(DelayedCommand.class);

private final Executor executor;

Expand Down Expand Up @@ -54,6 +62,15 @@ public void run()
try {
result = execute();
} catch (Exception e) {
try {
Method method = ReflectionUtils.getAnyMethod(getClass(), "execute");
if (!ReflectionUtils.hasDeclaredException(method, e)) {
LOGGER.error("Command failed due to a bug, please contact support@dcache.org.", e);
e = new CommandPanicException("Command failed: " + e.toString(), e);
}
} catch (NoSuchMethodException suppressed) {
e.addSuppressed(suppressed);
}
result = e;
}
reply(result);
Expand Down
Expand Up @@ -36,6 +36,7 @@
import dmg.util.command.PlainHelpPrinter;

import org.dcache.util.Args;
import org.dcache.util.ReflectionUtils;

import static com.google.common.collect.Iterables.*;
import static java.util.Arrays.asList;
Expand Down Expand Up @@ -135,15 +136,8 @@ public Serializable execute(Args arguments) throws CommandException
* those declared to be thrown by the method as
* bugs and propagate them.
*/
boolean declared = false;
Method method = command.getClass().getMethod("call");
for (Class<?> clazz: method.getExceptionTypes()) {
if (clazz.isAssignableFrom(e.getClass())) {
declared = true;
}
}

if (!declared) {
if (!ReflectionUtils.hasDeclaredException(method, e)) {
throw new CommandPanicException("Command failed: " + e.toString(), e);
}
throw new CommandThrowableException(
Expand Down
@@ -1,25 +1,16 @@
package org.dcache.util;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
import java.lang.reflect.Modifier;
import java.util.Arrays;
import java.util.HashMap;
import java.util.Map;

import diskCacheV111.util.PnfsId;

/**
* This class contains useful static methods for working with Java
* reflection.
*/
public class ReflectionUtils
{
private static final Logger _log = LoggerFactory.getLogger(ReflectionUtils.class);

private static final Map<String,Method> methodCache =
new HashMap<>();

Expand Down Expand Up @@ -84,32 +75,31 @@ public static Method resolve(Class<?> c, String name, Class<?> ... parameters)
}
}

public static boolean hasDeclaredException(Method method, Exception exception)
{
for (Class<?> clazz: method.getExceptionTypes()) {
if (clazz.isAssignableFrom(exception.getClass())) {
return true;
}
}
return false;
}

/**
* If <code>o</code> has a public getPnfsId method with an empty
* parameter list and a PnfsId return type, then the return value
* of that method is returned. Otherwise null is returned.
* Like Class#getMethod, but also returns non-public methods. Differs from
* Class#getDeclaredMethod by also searching super classes.
*/
public static PnfsId getPnfsId(Object o)
public static Method getAnyMethod(Class<?> clazz, String name, Class<?>... parameterTypes) throws NoSuchMethodException
{
try {
Class<?> c = o.getClass();
Method m = c.getMethod("getPnfsId");
if (PnfsId.class.isAssignableFrom(m.getReturnType()) &&
Modifier.isPublic(m.getModifiers())) {
m.setAccessible(true);
return (PnfsId)m.invoke(o);
}
// Because execute is protected, we cannot use getMethod.
return clazz.getDeclaredMethod(name, parameterTypes);
} catch (NoSuchMethodException e) {
// Not having a getPnfsId method is quite valid
} catch (IllegalAccessException e) {
// Having a non-public getPnfsId is unfortunate, but quite
// valid. Still we log it to better track the issue.
_log.debug("Failed to extract PNFS ID from object: "
+ e.getMessage(), e);
} catch (InvocationTargetException e) {
_log.error("Failed to extract PNFS ID from message: "
+ e.getMessage(), e);
Class<?> superclass = clazz.getSuperclass();
if (superclass != null) {
return getAnyMethod(superclass, name, parameterTypes);
}
throw e;
}
return null;
}
}

0 comments on commit 806e8c0

Please sign in to comment.