[LMS-2328] reopened + refixed

[LMS-2389] error handling improvements
1) Do not propagate exceptions from AbstractOmnscientTopLevelDataSetRegistrator to DirectoryWatcher - errors are dealt within the class itself when transactions are rolled back
2) Allow DssUploadClient to see exceptions that are not happening inside a transaction boundary (e.g. Jython scripts fails before it creates a transaction).

SVN: 22057

datastore_server/source/java/ch/systemsx/cisd/etlserver/registrator/AbstractOmniscientTopLevelDataSetRegistrator.java

@@ -26,10 +26,12 @@ import java.util.concurrent.locks.Lock;
 import java.util.concurrent.locks.ReentrantLock;
 
 import org.apache.log4j.Logger;
+import org.python.core.PyException;
 
 import ch.systemsx.cisd.base.exceptions.InterruptedExceptionUnchecked;
 import ch.systemsx.cisd.common.exceptions.ConfigurationFailureException;
 import ch.systemsx.cisd.common.exceptions.EnvironmentFailureException;
+import ch.systemsx.cisd.common.exceptions.UserFailureException;
 import ch.systemsx.cisd.common.filesystem.FileOperations;
 import ch.systemsx.cisd.common.filesystem.IFileOperations;
 import ch.systemsx.cisd.common.logging.LogCategory;
@@ -202,8 +204,6 @@ public abstract class AbstractOmniscientTopLevelDataSetRegistrator<T extends Dat
 
     private final OmniscientTopLevelDataSetRegistratorState state;
     
-    private final IThrowableHandler finalThrowableHandler;
-
     private boolean stopped;
 
     /**
@@ -213,25 +213,8 @@ public abstract class AbstractOmniscientTopLevelDataSetRegistrator<T extends Dat
      */
     protected AbstractOmniscientTopLevelDataSetRegistrator(
             TopLevelDataSetRegistratorGlobalState globalState)
-    {
-        this(globalState, new IThrowableHandler()
-            {
-                public void handle(Throwable throwable)
-                {
-                }
-            });
-    }
-
-    /**
-     * Constructor.
-     * 
-     * @param globalState
-     */
-    protected AbstractOmniscientTopLevelDataSetRegistrator(
-            TopLevelDataSetRegistratorGlobalState globalState, IThrowableHandler finalThrowableHandler)
     {
         super(globalState);
-        this.finalThrowableHandler = finalThrowableHandler;
 
         IStorageProcessorTransactional storageProcessor =
                 PropertiesBasedETLServerPlugin.create(IStorageProcessorTransactional.class,
@@ -322,9 +305,32 @@ public abstract class AbstractOmniscientTopLevelDataSetRegistrator<T extends Dat
                         new DoNothingDelegatedAction());
         if (service.didErrorsArise())
         {
+            Throwable firstError = service.getEncounteredErrors().get(0);
             throw new EnvironmentFailureException("Could not process file "
-                    + incomingDataSetFile.getName(), service.getEncounteredErrors().get(0));
+                    + incomingDataSetFile.getName(), serializableException(firstError));
+        }
+    }
+
+    /**
+     * Not all instances of PyExceptions are serializable, because they keep references to
+     * non-serializable objects e.g. java.lang.reflect.Method.
+     */
+    private Throwable serializableException(Throwable throwable)
+    {
+        if (throwable instanceof PyException)
+        {
+            return new RuntimeException(throwable.toString());
+        }
+        if (throwable instanceof UserFailureException)
+        {
+            return new RuntimeException(throwable.getMessage());
+        }
+        Throwable cause = throwable;
+        while (cause.getCause() != null)
+        {
+            cause = cause.getCause();
         }
+        return new RuntimeException(cause.toString());
     }
 
     /**
@@ -371,7 +377,6 @@ public abstract class AbstractOmniscientTopLevelDataSetRegistrator<T extends Dat
                             incomingDataSetFile, null, ex, ErrorType.REGISTRATION_SCRIPT_ERROR);
             operationLog.info(rollbacker.getErrorMessageForLog());
             rollbacker.doRollback();
-            finalThrowableHandler.handle(ex);
         }
 
         return service;
@@ -460,7 +465,7 @@ public abstract class AbstractOmniscientTopLevelDataSetRegistrator<T extends Dat
     {
         updateStopped(throwable instanceof InterruptedExceptionUnchecked);
 
-        service.abort();
+        service.abort(throwable);
     }
 
     /**

datastore_server/source/java/ch/systemsx/cisd/etlserver/registrator/DataSetRegistrationService.java

@@ -171,8 +171,9 @@ public class DataSetRegistrationService<T extends DataSetInformation> implements
     /**
      * Abort any scheduled changes.
      */
-    public void abort()
+    public void abort(Throwable t)
     {
+        encounteredErrors.add(t);
         rollbackExtantTransactions();
         dataSetRegistrations.clear();
     }

datastore_server/source/java/ch/systemsx/cisd/etlserver/registrator/IThrowableHandler.java

-/*
- * Copyright 2011 ETH Zuerich, CISD
- *
- * 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 ch.systemsx.cisd.etlserver.registrator;
-
-/**
- * Interface of classes able to handle {@link Throwable} instances.
- *
- * @author Franz-Josef Elmer
- */
-public interface IThrowableHandler
-{
-    public void handle(Throwable throwable);
-}

datastore_server/source/java/ch/systemsx/cisd/etlserver/registrator/JythonTopLevelDataSetHandler.java

@@ -19,12 +19,10 @@ package ch.systemsx.cisd.etlserver.registrator;
 import java.io.File;
 
 import org.python.core.Py;
-import org.python.core.PyException;
 import org.python.core.PyFunction;
 import org.python.util.PythonInterpreter;
 
 import ch.systemsx.cisd.common.exceptions.ConfigurationFailureException;
-import ch.systemsx.cisd.common.exceptions.UserFailureException;
 import ch.systemsx.cisd.common.filesystem.FileUtilities;
 import ch.systemsx.cisd.common.utilities.IDelegatedActionWithResult;
 import ch.systemsx.cisd.common.utilities.PropertyUtils;
@@ -77,27 +75,6 @@ public class JythonTopLevelDataSetHandler<T extends DataSetInformation> extends
 
     // The key for the script in the properties file
     public static final String SCRIPT_PATH_KEY = "script-path";
-    
-    private static final IThrowableHandler FINAL_THROWABLE_HANDLER = new IThrowableHandler()
-        {
-            public void handle(Throwable throwable)
-            {
-                if (throwable instanceof PyException)
-                {
-                    throw new RuntimeException(throwable.toString());
-                } 
-                if (throwable instanceof UserFailureException)
-                {
-                    throw new RuntimeException(throwable.getMessage());
-                }
-                Throwable cause = throwable;
-                while (cause.getCause() != null)
-                {
-                    cause = cause.getCause();
-                }
-                throw new RuntimeException(cause.toString());
-            }
-        };
 
     private final File scriptFile;
 
@@ -108,13 +85,7 @@ public class JythonTopLevelDataSetHandler<T extends DataSetInformation> extends
      */
     public JythonTopLevelDataSetHandler(TopLevelDataSetRegistratorGlobalState globalState)
     {
-        this(globalState, FINAL_THROWABLE_HANDLER);
-    }
-    
-    JythonTopLevelDataSetHandler(TopLevelDataSetRegistratorGlobalState globalState,
-            IThrowableHandler handler)
-    {
-        super(globalState, handler);
+        super(globalState);
 
         String path =
                 PropertyUtils.getMandatoryProperty(globalState.getThreadParameters()
@@ -145,18 +116,8 @@ public class JythonTopLevelDataSetHandler<T extends DataSetInformation> extends
         interpreter.set(STATE_VARIABLE_NAME, getGlobalState());
         interpreter.set(FACTORY_VARIABLE_NAME, service.getDataSetRegistrationDetailsFactory());
 
-        try
-        {
-            // Invoke the evaluator
-            interpreter.exec(scriptString);
-        } catch (Throwable ex)
-        {
-            operationLog
-                    .error(String
-                            .format("Cannot register dataset from a file '%s'. Error in jython dropbox has occured:\n%s",
-                                    dataSetFile.getPath(), ex.toString()));
-            throw ex;
-        }
+        // Invoke the evaluator
+        interpreter.exec(scriptString);
     }
 
     /**

datastore_server/sourceTest/java/ch/systemsx/cisd/etlserver/registrator/JythonTopLevelDataSetRegistratorTest.java

@@ -30,7 +30,6 @@ import org.apache.commons.io.FileUtils;
 import org.hamcrest.core.IsAnything;
 import org.jmock.Expectations;
 import org.jmock.Mockery;
-import org.python.core.PyException;
 import org.python.core.PyFunction;
 import org.python.util.PythonInterpreter;
 import org.testng.annotations.AfterMethod;
@@ -133,8 +132,6 @@ public class JythonTopLevelDataSetRegistratorTest extends AbstractFileSystemTest
 
     private BufferedAppender logAppender;
 
-    private IThrowableHandler throwableHandler;
-
     @BeforeTest
     public void init()
     {
@@ -156,7 +153,6 @@ public class JythonTopLevelDataSetRegistratorTest extends AbstractFileSystemTest
         context = new Mockery();
         openBisService = context.mock(IEncapsulatedOpenBISService.class);
         dataSetValidator = context.mock(IDataSetValidator.class);
-        throwableHandler = context.mock(IThrowableHandler.class);
         mailClient = context.mock(IMailClient.class);
 
         logAppender = new BufferedAppender();
@@ -703,7 +699,6 @@ public class JythonTopLevelDataSetRegistratorTest extends AbstractFileSystemTest
     public void testScriptDies()
     {
         setUpHomeDataBaseExpectations();
-        prepareThrowableHandling(PyException.class);
 
         Properties threadProperties =
                 createThreadPropertiesRelativeToScriptsFolder("dying-script.py");
@@ -738,7 +733,6 @@ public class JythonTopLevelDataSetRegistratorTest extends AbstractFileSystemTest
     public void testRollbackService()
     {
         setUpHomeDataBaseExpectations();
-        prepareThrowableHandling(PyException.class);
 
         // Create a handler that throws an exception during registration
         Properties threadProperties =
@@ -764,7 +758,6 @@ public class JythonTopLevelDataSetRegistratorTest extends AbstractFileSystemTest
     public void testScriptPathDeletedLater()
     {
         setUpHomeDataBaseExpectations();
-        prepareThrowableHandling(PyException.class);
         String scriptPath = "foo.py";
         Properties threadProperties = createThreadProperties(scriptPath);
 
@@ -830,12 +823,6 @@ public class JythonTopLevelDataSetRegistratorTest extends AbstractFileSystemTest
     public void testQuerying()
     {
         setUpHomeDataBaseExpectations();
-        context.checking(new Expectations()
-            {
-                {
-                    allowing(throwableHandler).handle(with(any(Exception.class)));
-                }
-            });
         Properties threadProperties =
                 createThreadPropertiesRelativeToScriptsFolder("query-interface-test.py");
         createHandler(threadProperties, false);
@@ -976,16 +963,6 @@ public class JythonTopLevelDataSetRegistratorTest extends AbstractFileSystemTest
                 }
             });
     }
-
-    private void prepareThrowableHandling(final Class<? extends Throwable> throwableClass)
-    {
-        context.checking(new Expectations()
-            {
-                {
-                    one(throwableHandler).handle(with(any(throwableClass)));
-                }
-            });
-    }
     
     private void setUpQueryExpectations()
     {
@@ -1113,7 +1090,7 @@ public class JythonTopLevelDataSetRegistratorTest extends AbstractFileSystemTest
         public TestingDataSetHandler(TopLevelDataSetRegistratorGlobalState globalState,
                 boolean shouldRegistrationFail, boolean shouldReThrowRollbackException)
         {
-            super(globalState, throwableHandler);
+            super(globalState);
             this.shouldRegistrationFail = shouldRegistrationFail;
             this.shouldReThrowRollbackException = shouldReThrowRollbackException;
         }