From e078737493dddbee6047d912f334d1755b1597f0 Mon Sep 17 00:00:00 2001 From: Scott M Stark Date: Tue, 17 Dec 2024 16:25:36 -0600 Subject: [PATCH] Step 1 of improving the handling of exception propagation Signed-off-by: Scott M Stark --- test/spi/README.md | 6 + test/spi/pom.xml | 45 +++++- .../test/spi/ArquillianProxyException.java | 9 +- .../arquillian/test/spi/ExceptionProxy.java | 143 ++++++++++++++++-- .../test/spi/ExceptionProxyTestCase.java | 50 +++++- .../org/jboss/arquillian/test/spi/IBean.java | 5 + .../jboss/arquillian/test/spi/IException.java | 12 ++ .../test/spi/serveronly/SomeBean.java | 20 +++ .../SomeNonClientSideException.java | 10 ++ 9 files changed, 283 insertions(+), 17 deletions(-) create mode 100644 test/spi/README.md create mode 100644 test/spi/src/test/java/org/jboss/arquillian/test/spi/IBean.java create mode 100644 test/spi/src/test/java/org/jboss/arquillian/test/spi/IException.java create mode 100644 test/spi/src/test/java/org/jboss/arquillian/test/spi/serveronly/SomeBean.java create mode 100644 test/spi/src/test/java/org/jboss/arquillian/test/spi/serveronly/SomeNonClientSideException.java diff --git a/test/spi/README.md b/test/spi/README.md new file mode 100644 index 000000000..4bd05f6f1 --- /dev/null +++ b/test/spi/README.md @@ -0,0 +1,6 @@ +The tests in this module use a custom maven compiler configuration to simulate a class that exists on the server side but not on the client side to validate the behavior seen in issue +https://github.com/arquillian/arquillian-core/issues/641. + +To be able to run these tests from within Intellij, one has to configure the +"Settings | Build, Execution, Deployment | Build Tools | Maven | Runner" to have the "Delegate IDE build/run actions to Maven" box checked. + diff --git a/test/spi/pom.xml b/test/spi/pom.xml index bbafb3bd2..c09f6e8b0 100644 --- a/test/spi/pom.xml +++ b/test/spi/pom.xml @@ -1,5 +1,7 @@ - + @@ -53,5 +55,46 @@ + + + + + + org.apache.maven.plugins + maven-compiler-plugin + + + + default-testCompile + test-compile + + testCompile + + + + org/jboss/arquillian/test/spi/serveronly/** + + + + + + serveronly-compile + test-compile + + testCompile + + + + org/jboss/arquillian/test/spi/serveronly/** + + ${project.build.directory}/serveronly-classes + + + + + + diff --git a/test/spi/src/main/java/org/jboss/arquillian/test/spi/ArquillianProxyException.java b/test/spi/src/main/java/org/jboss/arquillian/test/spi/ArquillianProxyException.java index 5681efb99..ff0a6ae7a 100644 --- a/test/spi/src/main/java/org/jboss/arquillian/test/spi/ArquillianProxyException.java +++ b/test/spi/src/main/java/org/jboss/arquillian/test/spi/ArquillianProxyException.java @@ -16,9 +16,11 @@ */ package org.jboss.arquillian.test.spi; +import java.io.PrintStream; + /** * Exception class used when a proxied exception cannot be created. This - * exception type is is thrown instead and contains information about the + * exception type is thrown instead and contains information about the * proxied class and a hint about why it could not be thrown. * * @author Andy Gibson @@ -59,4 +61,9 @@ public ArquillianProxyException(Throwable cause) { public ArquillianProxyException(String message, String exceptionClassName, String reason, Throwable cause) { this(String.format("%s : %s [Proxied because : %s]", exceptionClassName, message, reason), cause); } + + @Override + public void printStackTrace(PrintStream s) { + super.printStackTrace(s); + } } diff --git a/test/spi/src/main/java/org/jboss/arquillian/test/spi/ExceptionProxy.java b/test/spi/src/main/java/org/jboss/arquillian/test/spi/ExceptionProxy.java index 7e6fca9e3..32c7e58e3 100644 --- a/test/spi/src/main/java/org/jboss/arquillian/test/spi/ExceptionProxy.java +++ b/test/spi/src/main/java/org/jboss/arquillian/test/spi/ExceptionProxy.java @@ -25,7 +25,10 @@ import java.io.ObjectInputStream; import java.io.ObjectOutput; import java.io.ObjectOutputStream; +import java.io.Serializable; import java.lang.reflect.InvocationTargetException; +import java.util.ArrayList; +import java.util.List; /** * Takes an exception class and creates a proxy that can be used to rebuild the @@ -50,32 +53,43 @@ * @author Andy Gibson */ public class ExceptionProxy implements Externalizable { - + // The serialVersionUID of the ExceptionProxy that existed in Arquillian 1.9.1.Final private static final long serialVersionUID = 2321010311438950147L; + // This is the className of the exception in the container passed into TestResult#setThrowable(Throwable) private String className; - + // This is the message of the exception in the container passed into TestResult#setThrowable(Throwable) private String message; - + // This is the stack trace of the exception in the container passed into TestResult#setThrowable(Throwable) private StackTraceElement[] trace; - + // This is a proxy to the cause exception in the container, not used post 1.9.1.Final private ExceptionProxy causeProxy; - + // This is the causeProxy#createException() instance private Throwable cause; - + // This only exists if the original container exception could be deserialized in the client private Throwable original; - + // This would exist if the original exception could not be serialized in the container private Throwable serializationProcessException = null; + // New fields added in 1.9.2.Final + private Version version; + private List causeHierarchy; + + public static class Version implements Serializable { + int version = 2; + } public ExceptionProxy() { + version = new Version(); } public ExceptionProxy(Throwable throwable) { + this.version = new Version(); this.className = throwable.getClass().getName(); this.message = throwable.getMessage(); this.trace = throwable.getStackTrace(); - this.causeProxy = ExceptionProxy.createForException(throwable.getCause()); + //this.causeProxy = ExceptionProxy.createForException(throwable.getCause()); this.original = throwable; + this.causeHierarchy = getExceptionHierarchy(throwable); } /** @@ -106,7 +120,8 @@ public boolean hasException() { /** * Constructs an instance of the proxied exception based on the class name, - * message, stack trace and if applicable, the cause. + * message, stack trace and if applicable, and the cause if the cause could be + * deserialized in the client. Otherwise, this returns an ArquillianProxyException * * @return The constructed {@link Throwable} instance */ @@ -143,7 +158,7 @@ public Throwable getCause() { cause = causeProxy.createException(); } } - return cause; + return serializationProcessException; } /** @@ -162,7 +177,44 @@ public Throwable getCause() { */ @Override public void readExternal(ObjectInput in) throws IOException, ClassNotFoundException { - className = (String) in.readObject(); + // Read the first object to see if it is the version object + Object firstObject = in.readObject(); + if (firstObject instanceof Version) { + version = (Version) firstObject; + className = (String) in.readObject(); + message = (String) in.readObject(); + trace = (StackTraceElement[]) in.readObject(); + causeHierarchy = (List) in.readObject(); + // Try to deserialize the original exception + try { + byte[] originalExceptionData = (byte[]) in.readObject(); + if (originalExceptionData != null && originalExceptionData.length > 0) { + ByteArrayInputStream originalIn = new ByteArrayInputStream(originalExceptionData); + ObjectInputStream input = new ObjectInputStream(originalIn); + original = (Throwable) input.readObject(); + } + } catch (Throwable e) { + this.serializationProcessException = e; + } + // Override with the remote serialization issue cause if exists + Throwable tmpSerializationProcessException = (Throwable) in.readObject(); + if (tmpSerializationProcessException != null) { + serializationProcessException = tmpSerializationProcessException; + } + + // + if(serializationProcessException == null && original == null) { + original = buildOriginalException(); + } + } else { + // If it is not the version object this is an old version of the ExceptionProxy + readExternal_191Final((String) firstObject, in); + } + } + + // No longer used in 1.9.2.Final+, can be removed in 2.0.0.Final + protected void readExternal_191Final(String className, ObjectInput in) throws IOException, ClassNotFoundException { + this.className = className; message = (String) in.readObject(); trace = (StackTraceElement[]) in.readObject(); causeProxy = (ExceptionProxy) in.readObject(); @@ -208,6 +260,32 @@ public void readExternal(ObjectInput in) throws IOException, ClassNotFoundExcept @Override public void writeExternal(ObjectOutput out) throws IOException { + out.writeObject(version); + out.writeObject(className); + out.writeObject(message); + out.writeObject(trace); + out.writeObject(causeHierarchy); + byte[] originalBytes = new byte[0]; + try { + /* Try to serialize the original exception. Here we do it in a separate try-catch block to avoid + because default serialization will serialize whatever it can and leave non-serializable fields out. + We have to make the write of the root exception atomic. + */ + ByteArrayOutputStream originalOut = new ByteArrayOutputStream(); + ObjectOutputStream output = new ObjectOutputStream(originalOut); + output.writeObject(original); + output.flush(); + originalBytes = originalOut.toByteArray(); + } catch (NotSerializableException e) { + // ignore, could not serialize original exception + this.serializationProcessException = e; + } + out.writeObject(originalBytes); + out.writeObject(serializationProcessException); + } + + // No longer used in 1.9.2.Final+, can be removed in 2.0.0.Final + protected void writeExternal_191Final(ObjectOutput out) throws IOException { out.writeObject(className); out.writeObject(message); out.writeObject(trace); @@ -241,4 +319,47 @@ public void writeExternal(ObjectOutput out) throws IOException { public String toString() { return super.toString() + String.format("[class=%s, message=%s],cause = %s", className, message, causeProxy); } + + /** + * Get the exception hierarchy for the exception class + * + * @return list of exception types in the hierarchy + */ + protected List getExceptionHierarchy(Throwable t) { + List hierarchy = new ArrayList<>(); + Class tclass = t.getClass(); + while(Throwable.class.isAssignableFrom(tclass)) { + hierarchy.add(tclass.getName()); + tclass = tclass.getSuperclass(); + } + return hierarchy; + } + /** + * Build the original exception based on the exception class name. This first + * tries to use a ctor with a message, then a default ctor. + * + * @return the original exception + */ + protected Throwable buildOriginalException() { + Throwable original = null; + for(String tclassName : causeHierarchy) { + try { + Class tclass = Class.forName(tclassName).asSubclass(Throwable.class); + try { + original = tclass.getDeclaredConstructor(String.class).newInstance(message); + break; + } catch (Exception e) { + try { + original = tclass.getDeclaredConstructor().newInstance(); + break; + } catch (Exception ex) { + // ignore, could not load class on client side, try next base class + } + } + } catch (ClassNotFoundException e) { + // ignore, could not load class on client side, try next base class + } + } + return original; + } } diff --git a/test/spi/src/test/java/org/jboss/arquillian/test/spi/ExceptionProxyTestCase.java b/test/spi/src/test/java/org/jboss/arquillian/test/spi/ExceptionProxyTestCase.java index 608c9094c..833cd7fa4 100644 --- a/test/spi/src/test/java/org/jboss/arquillian/test/spi/ExceptionProxyTestCase.java +++ b/test/spi/src/test/java/org/jboss/arquillian/test/spi/ExceptionProxyTestCase.java @@ -20,6 +20,7 @@ import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; import java.io.Externalizable; +import java.io.File; import java.io.IOException; import java.io.InputStream; import java.io.ObjectInput; @@ -28,11 +29,17 @@ import java.io.ObjectOutputStream; import java.lang.reflect.Constructor; import java.lang.reflect.InvocationTargetException; +import java.net.URL; +import java.net.URLClassLoader; + import org.junit.Assert; import org.junit.Test; /** * ExceptionProxyTestCase + * Updated for https://github.com/arquillian/arquillian-core/issues/641 + * where the exception seen by a client that did not have the exception class + * thrown from a server was not on the client classpath. * * @author Aslak Knutsen * @version $Revision: $ @@ -67,7 +74,6 @@ public void shouldSerializeNonSerializableExceptions() throws Exception { Assert.assertTrue( "Verify Proxy message contain root cause of serialization problem", t.getMessage().contains("BufferedInputStream")); - Assert.assertEquals(UnsupportedOperationException.class, t.getCause().getClass()); } @Test @@ -82,7 +88,8 @@ public void shouldSerializeNonDeSerializableExceptions() throws Exception { Assert.assertTrue( "Verify Proxy message contain root cause of deserialization problem", t.getMessage().contains("Could not de-serialize")); - Assert.assertEquals(UnsupportedOperationException.class, t.getCause().getClass()); + // This is not valid if the exception is not serializable + //Assert.assertEquals(UnsupportedOperationException.class, t.getCause().getClass()); } @Test @@ -96,12 +103,44 @@ public void shouldRecreateInvocationTargetExceptions() throws Exception { Assert.assertEquals(ClassNotFoundException.class, t.getCause().getCause().getClass()); } + @Test + public void handleExceptionClassNotOnClientClasspath() throws Throwable { + Throwable serverException = causeServerException(); + System.out.println("Loaded server exception: " + serverException); + ExceptionProxy proxy = serialize(ExceptionProxy.createForException(serverException)); + Throwable t = proxy.createException(); + System.out.println("Client exception from proxy: " + t); + System.out.println("Client exception trace from proxy:"); + t.printStackTrace(); + Assert.assertEquals(ArquillianProxyException.class, t.getClass()); + Assert.assertEquals(ClassNotFoundException.class, t.getCause().getClass()); + } + + private Throwable causeServerException() throws Exception { + // Create a ClassLoader for the target/serveronly-classes dir + File serverOnlyClasses = new File("target/serveronly-classes"); + Assert.assertTrue("target/serveronly-classes should exist", serverOnlyClasses.exists()); + URL[] serveronlyCP = {serverOnlyClasses.toURL()}; + URLClassLoader classLoader = new URLClassLoader(serveronlyCP, getClass().getClassLoader()); + Class exClass = (Class) classLoader.loadClass("org.jboss.arquillian.test.spi.serveronly.SomeBean"); + IBean bean = exClass.newInstance(); + Throwable exception = null; + try { + bean.invoke(); + } catch (Exception e) { + exception = e; + } + return exception; + } + private ExceptionProxy serialize(ExceptionProxy proxy) throws Exception { ByteArrayOutputStream output = new ByteArrayOutputStream(); ObjectOutputStream out = new ObjectOutputStream(output); out.writeObject(proxy); + out.close(); + byte[] data = output.toByteArray(); - ObjectInputStream in = new ObjectInputStream(new ByteArrayInputStream(output.toByteArray())); + ObjectInputStream in = new ObjectInputStream(new ByteArrayInputStream(data)); return (ExceptionProxy) in.readObject(); } @@ -122,7 +161,10 @@ private void printConstructors(Throwable throwable) throws Exception { } } - // Simulate org.jboss.weld.exceptions.IllegalArgumentException + /** Simulate org.jboss.weld.exceptions.IllegalArgumentException + * Note, this does not simulate the case of weld implementation classes not + * being on the test client classpath, which is the norm. + */ private static class ExtendedIllegalArgumentException extends IllegalArgumentException { private static final long serialVersionUID = 1L; diff --git a/test/spi/src/test/java/org/jboss/arquillian/test/spi/IBean.java b/test/spi/src/test/java/org/jboss/arquillian/test/spi/IBean.java new file mode 100644 index 000000000..6efbaed9a --- /dev/null +++ b/test/spi/src/test/java/org/jboss/arquillian/test/spi/IBean.java @@ -0,0 +1,5 @@ +package org.jboss.arquillian.test.spi; + +public interface IBean { + String invoke(); +} diff --git a/test/spi/src/test/java/org/jboss/arquillian/test/spi/IException.java b/test/spi/src/test/java/org/jboss/arquillian/test/spi/IException.java new file mode 100644 index 000000000..727a2731f --- /dev/null +++ b/test/spi/src/test/java/org/jboss/arquillian/test/spi/IException.java @@ -0,0 +1,12 @@ +package org.jboss.arquillian.test.spi; + +/** + * An exception class that is common to both the client and server container side. + */ +public class IException extends RuntimeException { + private static final long serialVersionUID = 1L; + + public IException(String message) { + super(message); + } +} diff --git a/test/spi/src/test/java/org/jboss/arquillian/test/spi/serveronly/SomeBean.java b/test/spi/src/test/java/org/jboss/arquillian/test/spi/serveronly/SomeBean.java new file mode 100644 index 000000000..227de7e92 --- /dev/null +++ b/test/spi/src/test/java/org/jboss/arquillian/test/spi/serveronly/SomeBean.java @@ -0,0 +1,20 @@ +package org.jboss.arquillian.test.spi.serveronly; + +import org.jboss.arquillian.test.spi.IBean; + +/** + * A simple bean that throws an exception. This is loaded by a custom classloader + * in the test client so that the exception thrown is not available on the client. + */ +public class SomeBean implements IBean { + public String invoke() { + return invokeImpl(); + } + + private String invokeImpl() { + return invokeImplNested(); + } + private String invokeImplNested() throws SomeNonClientSideException { + throw new SomeNonClientSideException(); + } +} diff --git a/test/spi/src/test/java/org/jboss/arquillian/test/spi/serveronly/SomeNonClientSideException.java b/test/spi/src/test/java/org/jboss/arquillian/test/spi/serveronly/SomeNonClientSideException.java new file mode 100644 index 000000000..1a80cf954 --- /dev/null +++ b/test/spi/src/test/java/org/jboss/arquillian/test/spi/serveronly/SomeNonClientSideException.java @@ -0,0 +1,10 @@ +package org.jboss.arquillian.test.spi.serveronly; + +import org.jboss.arquillian.test.spi.IException; + +public class SomeNonClientSideException extends IException { + private static final long serialVersionUID = 1L; + public SomeNonClientSideException() { + super("The server side reason for this exception"); + } +}