This is an automated email from the ASF dual-hosted git repository. chaokunyang pushed a commit to branch releases-0.12 in repository https://gitbox.apache.org/repos/asf/fory.git
commit ab49713470bebebab814908d6c2223bae1b62136 Author: Shawn Yang <[email protected]> AuthorDate: Thu Sep 4 15:10:48 2025 +0800 refactor(java): refactor fory java exception hierarchical structure (#2577) <!-- **Thanks for contributing to Fory.** **If this is your first time opening a PR on fory, you can refer to [CONTRIBUTING.md](https://github.com/apache/fory/blob/main/CONTRIBUTING.md).** Contribution Checklist - The **Apache Fory** community has requirements on the naming of pr titles. You can also find instructions in [CONTRIBUTING.md](https://github.com/apache/fory/blob/main/CONTRIBUTING.md). - Fory has a strong focus on performance. If the PR you submit will have an impact on performance, please benchmark it first and provide the benchmark result here. --> ## Why? <!-- Describe the purpose of this PR. --> ## What does this PR do? <!-- Describe the details of this PR. --> ## Related issues <!-- Is there any related issue? If this PR closes them you say say fix/closes: - #xxxx0 - #xxxx1 - Fixes #xxxx2 --> ## Does this PR introduce any user-facing change? <!-- If any user-facing interface changes, please [open an issue](https://github.com/apache/fory/issues/new/choose) describing the need to do so and update the document if necessary. Delete section if not applicable. --> - [ ] Does this PR introduce any public API change? - [ ] Does this PR introduce any binary protocol compatibility change? ## Benchmark <!-- When the PR has an impact on performance (if you don't know whether the PR will have an impact on performance, you can submit the PR first, and if it will have impact on performance, the code reviewer will explain it), be sure to attach a benchmark data here. Delete section if not applicable. --> --- .../src/main/java/org/apache/fory/Fory.java | 50 ++++++++++++---------- .../org/apache/fory/exception/CopyException.java | 36 ++++++++++++++++ .../fory/exception/DeserializationException.java | 1 + .../fory/exception/SerializationException.java | 36 ++++++++++++++++ .../java/org/apache/fory/util/ExceptionUtils.java | 12 +++--- .../src/test/java/org/apache/fory/ForyTest.java | 13 +++--- .../test/java/org/apache/fory/ForyTestBase.java | 23 ++++++++++ .../org/apache/fory/resolver/MetaContextTest.java | 2 +- .../fory/serializer/LambdaSerializerTest.java | 3 +- .../NonexistentClassSerializersTest.java | 2 +- .../collection/CollectionSerializersTest.java | 4 +- .../serializer/collection/MapSerializersTest.java | 8 ++-- 12 files changed, 147 insertions(+), 43 deletions(-) diff --git a/java/fory-core/src/main/java/org/apache/fory/Fory.java b/java/fory-core/src/main/java/org/apache/fory/Fory.java index 5bacf507f..1786f8c95 100644 --- a/java/fory-core/src/main/java/org/apache/fory/Fory.java +++ b/java/fory-core/src/main/java/org/apache/fory/Fory.java @@ -37,6 +37,10 @@ import org.apache.fory.config.Config; import org.apache.fory.config.ForyBuilder; import org.apache.fory.config.Language; import org.apache.fory.config.LongEncoding; +import org.apache.fory.exception.CopyException; +import org.apache.fory.exception.DeserializationException; +import org.apache.fory.exception.ForyException; +import org.apache.fory.exception.SerializationException; import org.apache.fory.io.ForyInputStream; import org.apache.fory.io.ForyReadableChannel; import org.apache.fory.logging.Logger; @@ -327,8 +331,8 @@ public final class Fory implements BaseFory { xwrite(buffer, obj); } return buffer; - } catch (StackOverflowError t) { - throw processStackOverflowError(t); + } catch (Throwable t) { + throw processSerializationError(t); } finally { resetWrite(); jitContext.unlock(); @@ -345,7 +349,7 @@ public final class Fory implements BaseFory { serializeToStream(outputStream, buf -> serialize(buf, obj, callback)); } - private StackOverflowError processStackOverflowError(StackOverflowError e) { + private ForyException processSerializationError(Throwable e) { if (!refTracking) { String msg = "Object may contain circular references, please enable ref tracking " @@ -354,25 +358,27 @@ public final class Fory implements BaseFory { if (StringUtils.isNotBlank(rawMessage)) { msg += ": " + rawMessage; } - StackOverflowError t1 = ExceptionUtils.trySetStackOverflowErrorMessage(e, msg); - if (t1 != null) { - return t1; + if (e instanceof StackOverflowError) { + e = ExceptionUtils.trySetStackOverflowErrorMessage((StackOverflowError) e, msg); } } - throw e; + if (!(e instanceof ForyException)) { + e = new SerializationException(e); + } + throw (ForyException) e; } - private StackOverflowError processCopyStackOverflowError(StackOverflowError e) { + private ForyException processCopyError(Throwable e) { if (!copyRefTracking) { String msg = "Object may contain circular references, please enable ref tracking " + "by `ForyBuilder#withRefCopy(true)`"; - StackOverflowError t1 = ExceptionUtils.trySetStackOverflowErrorMessage(e, msg); - if (t1 != null) { - return t1; - } + e = ExceptionUtils.trySetStackOverflowErrorMessage((StackOverflowError) e, msg); } - throw e; + if (!(e instanceof ForyException)) { + throw new CopyException(e); + } + throw (ForyException) e; } public MemoryBuffer getBuffer() { @@ -1188,8 +1194,8 @@ public final class Fory implements BaseFory { writeData(buffer, classInfo, obj); } } - } catch (StackOverflowError t) { - throw processStackOverflowError(t); + } catch (Throwable t) { + throw processSerializationError(t); } finally { resetWrite(); jitContext.unlock(); @@ -1304,8 +1310,8 @@ public final class Fory implements BaseFory { throwDepthSerializationException(); } write(buffer, obj); - } catch (StackOverflowError t) { - throw processStackOverflowError(t); + } catch (Throwable t) { + throw processSerializationError(t); } finally { resetWrite(); jitContext.unlock(); @@ -1384,8 +1390,8 @@ public final class Fory implements BaseFory { public <T> T copy(T obj) { try { return copyObject(obj); - } catch (StackOverflowError e) { - throw processCopyStackOverflowError(e); + } catch (Throwable e) { + throw processCopyError(e); } finally { if (copyRefTracking) { resetCopy(); @@ -1550,7 +1556,7 @@ public final class Fory implements BaseFory { } outputStream.flush(); } catch (IOException e) { - throw new RuntimeException(e); + throw new SerializationException(e); } finally { resetBuffer(); } @@ -1606,7 +1612,7 @@ public final class Fory implements BaseFory { private void throwDepthSerializationException() { String method = "Fory#" + (crossLanguage ? "x" : "") + "writeXXX"; - throw new IllegalStateException( + throw new SerializationException( String.format( "Nested call Fory.serializeXXX is not allowed when serializing, Please use %s instead", method)); @@ -1614,7 +1620,7 @@ public final class Fory implements BaseFory { private void throwDepthDeserializationException() { String method = "Fory#" + (crossLanguage ? "x" : "") + "readXXX"; - throw new IllegalStateException( + throw new DeserializationException( String.format( "Nested call Fory.deserializeXXX is not allowed when deserializing, Please use %s instead", method)); diff --git a/java/fory-core/src/main/java/org/apache/fory/exception/CopyException.java b/java/fory-core/src/main/java/org/apache/fory/exception/CopyException.java new file mode 100644 index 000000000..c64e04e77 --- /dev/null +++ b/java/fory-core/src/main/java/org/apache/fory/exception/CopyException.java @@ -0,0 +1,36 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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 org.apache.fory.exception; + +/** Exception thrown when a copy operation fails. */ +public class CopyException extends ForyException { + + public CopyException(String message) { + super(message); + } + + public CopyException(Throwable cause) { + super(cause); + } + + public CopyException(String message, Throwable cause) { + super(message, cause); + } +} diff --git a/java/fory-core/src/main/java/org/apache/fory/exception/DeserializationException.java b/java/fory-core/src/main/java/org/apache/fory/exception/DeserializationException.java index 1afe56088..1d6099ecb 100644 --- a/java/fory-core/src/main/java/org/apache/fory/exception/DeserializationException.java +++ b/java/fory-core/src/main/java/org/apache/fory/exception/DeserializationException.java @@ -21,6 +21,7 @@ package org.apache.fory.exception; import java.util.List; +/** Exception thrown when a deserialization operation fails. */ public class DeserializationException extends ForyException { private List<Object> readObjects; diff --git a/java/fory-core/src/main/java/org/apache/fory/exception/SerializationException.java b/java/fory-core/src/main/java/org/apache/fory/exception/SerializationException.java new file mode 100644 index 000000000..381e60d4e --- /dev/null +++ b/java/fory-core/src/main/java/org/apache/fory/exception/SerializationException.java @@ -0,0 +1,36 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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 org.apache.fory.exception; + +/** Exception thrown when a copy operation fails. */ +public class SerializationException extends ForyException { + + public SerializationException(String message) { + super(message); + } + + public SerializationException(Throwable cause) { + super(cause); + } + + public SerializationException(String message, Throwable cause) { + super(message, cause); + } +} diff --git a/java/fory-core/src/main/java/org/apache/fory/util/ExceptionUtils.java b/java/fory-core/src/main/java/org/apache/fory/util/ExceptionUtils.java index 51adcd671..5e08141dc 100644 --- a/java/fory-core/src/main/java/org/apache/fory/util/ExceptionUtils.java +++ b/java/fory-core/src/main/java/org/apache/fory/util/ExceptionUtils.java @@ -26,7 +26,6 @@ import org.apache.fory.Fory; import org.apache.fory.collection.ObjectArray; import org.apache.fory.exception.DeserializationException; import org.apache.fory.exception.ForyException; -import org.apache.fory.memory.Platform; import org.apache.fory.reflect.ReflectionUtils; import org.apache.fory.resolver.MapRefResolver; @@ -52,7 +51,7 @@ public class ExceptionUtils { ReflectionUtils.setObjectFieldValue(e, detailMessageField, message); return e; } else { - return null; + return e; } } @@ -62,11 +61,12 @@ public class ExceptionUtils { // carry with read objects for better trouble shooting. List<Object> objects = Arrays.asList(readObjects.objects).subList(0, readObjects.size); throw new DeserializationException(objects, t); - } else if (t instanceof Exception && !(t instanceof ForyException)) { - throw new DeserializationException("Failed to deserialize input", t); } else { - Platform.throwException(t); - throw new IllegalStateException("unreachable"); + if (!(t instanceof ForyException)) { + throw new DeserializationException("Failed to deserialize input", t); + } else { + throw (ForyException) t; + } } } diff --git a/java/fory-core/src/test/java/org/apache/fory/ForyTest.java b/java/fory-core/src/test/java/org/apache/fory/ForyTest.java index 335cc9720..cbb6fb32c 100644 --- a/java/fory-core/src/test/java/org/apache/fory/ForyTest.java +++ b/java/fory-core/src/test/java/org/apache/fory/ForyTest.java @@ -61,6 +61,7 @@ import org.apache.fory.config.Language; import org.apache.fory.exception.DeserializationException; import org.apache.fory.exception.ForyException; import org.apache.fory.exception.InsecureException; +import org.apache.fory.exception.SerializationException; import org.apache.fory.memory.MemoryBuffer; import org.apache.fory.memory.MemoryUtils; import org.apache.fory.memory.Platform; @@ -415,7 +416,8 @@ public class ForyTest extends ForyTestBase { serDe(fory, ByteBuffer.allocate(32)); serDe(fory, ByteBuffer.allocateDirect(32)); assertThrows(InsecureException.class, () -> fory.serialize(new Thread())); - assertThrows(UnsupportedOperationException.class, () -> fory.serialize(MethodHandles.lookup())); + assertThrowsCause( + UnsupportedOperationException.class, () -> fory.serialize(MethodHandles.lookup())); } @Test @@ -478,7 +480,7 @@ public class ForyTest extends ForyTestBase { @Test public void testExposeFields2() { Fory fory = Fory.builder().requireClassRegistration(false).build(); - assertThrows(RuntimeException.class, () -> serDe(fory, new ExposeFields2(1, 2, 3))); + assertThrowsCause(RuntimeException.class, () -> serDe(fory, new ExposeFields2(1, 2, 3))); } @Test(timeOut = 60_000) @@ -565,9 +567,10 @@ public class ForyTest extends ForyTestBase { Fory fory = Fory.builder().withRefTracking(false).requireClassRegistration(false).build(); try { fory.serialize(a); - throw new IllegalStateException("StackOverflowError not raised."); - } catch (StackOverflowError e) { - Assert.assertTrue(e.getMessage().contains("reference")); + throw new IllegalStateException("SerializationException not raised."); + } catch (SerializationException e) { + Throwable ex = e.getCause(); + Assert.assertTrue(ex.getMessage().contains("reference")); } } diff --git a/java/fory-core/src/test/java/org/apache/fory/ForyTestBase.java b/java/fory-core/src/test/java/org/apache/fory/ForyTestBase.java index f2ac457c7..6e156fce7 100644 --- a/java/fory-core/src/test/java/org/apache/fory/ForyTestBase.java +++ b/java/fory-core/src/test/java/org/apache/fory/ForyTestBase.java @@ -44,6 +44,7 @@ import org.apache.fory.reflect.ReflectionUtils; import org.apache.fory.resolver.MetaContext; import org.apache.fory.serializer.BufferObject; import org.testng.Assert; +import org.testng.Assert.ThrowingRunnable; import org.testng.annotations.DataProvider; /** Fory unit test base class. */ @@ -427,4 +428,26 @@ public abstract class ForyTestBase { int depth = Platform.getInt(fory, offset); Platform.putInt(fory, offset, depth + diff); } + + public static <T extends Throwable> void assertThrowsCause( + Class<T> throwableClass, ThrowingRunnable runnable) { + try { + runnable.run(); + } catch (Throwable t) { + Throwable cause = t.getCause(); + Assert.assertNotNull(cause); + if (throwableClass.isInstance(cause)) { + return; + } else { + throw new AssertionError( + String.format( + "Expected %s to be thrown, but %s was thrown", + throwableClass.getSimpleName(), cause.getClass().getSimpleName()), + cause); + } + } + throw new AssertionError( + String.format( + "Expected %s to be thrown, but nothing was thrown", throwableClass.getSimpleName())); + } } diff --git a/java/fory-core/src/test/java/org/apache/fory/resolver/MetaContextTest.java b/java/fory-core/src/test/java/org/apache/fory/resolver/MetaContextTest.java index 4b9aac1f0..054d66f84 100644 --- a/java/fory-core/src/test/java/org/apache/fory/resolver/MetaContextTest.java +++ b/java/fory-core/src/test/java/org/apache/fory/resolver/MetaContextTest.java @@ -79,7 +79,7 @@ public class MetaContextTest extends ForyTestBase { Assert.assertEquals(fory.deserialize(bytes1), o); fory.getSerializationContext().setMetaContext(new MetaContext()); Assert.assertEquals(fory.serialize(o), bytes); - Assert.assertThrows(AssertionError.class, () -> fory.serialize(o)); + assertThrowsCause(AssertionError.class, () -> fory.serialize(o)); } // final InnerPojo will be taken as non-final for writing class def. diff --git a/java/fory-core/src/test/java/org/apache/fory/serializer/LambdaSerializerTest.java b/java/fory-core/src/test/java/org/apache/fory/serializer/LambdaSerializerTest.java index 3bbd4c820..356e81e19 100644 --- a/java/fory-core/src/test/java/org/apache/fory/serializer/LambdaSerializerTest.java +++ b/java/fory-core/src/test/java/org/apache/fory/serializer/LambdaSerializerTest.java @@ -21,7 +21,6 @@ package org.apache.fory.serializer; import static org.testng.Assert.assertEquals; import static org.testng.Assert.assertSame; -import static org.testng.Assert.assertThrows; import static org.testng.Assert.assertTrue; import java.io.Serializable; @@ -81,7 +80,7 @@ public class LambdaSerializerTest extends ForyTestBase { public void testLambdaUnserializableMsg() { Fory fory = Fory.builder().requireClassRegistration(false).build(); Function<Object, String> function = String::valueOf; - assertThrows(UnsupportedOperationException.class, () -> fory.serialize(function)); + assertThrowsCause(UnsupportedOperationException.class, () -> fory.serialize(function)); try { fory.serialize(function); } catch (Exception e) { diff --git a/java/fory-core/src/test/java/org/apache/fory/serializer/NonexistentClassSerializersTest.java b/java/fory-core/src/test/java/org/apache/fory/serializer/NonexistentClassSerializersTest.java index c9ec51cbd..b75921b97 100644 --- a/java/fory-core/src/test/java/org/apache/fory/serializer/NonexistentClassSerializersTest.java +++ b/java/fory-core/src/test/java/org/apache/fory/serializer/NonexistentClassSerializersTest.java @@ -376,6 +376,6 @@ public class NonexistentClassSerializersTest extends ForyTestBase { .withClassLoader(classLoader) .build(); byte[] bytes = fory.serialize(pojo); - Assert.assertThrows(RuntimeException.class, () -> fory2.deserialize(bytes)); + assertThrowsCause(RuntimeException.class, () -> fory2.deserialize(bytes)); } } diff --git a/java/fory-core/src/test/java/org/apache/fory/serializer/collection/CollectionSerializersTest.java b/java/fory-core/src/test/java/org/apache/fory/serializer/collection/CollectionSerializersTest.java index d2fcc0a1a..211679c98 100644 --- a/java/fory-core/src/test/java/org/apache/fory/serializer/collection/CollectionSerializersTest.java +++ b/java/fory-core/src/test/java/org/apache/fory/serializer/collection/CollectionSerializersTest.java @@ -158,7 +158,7 @@ public class CollectionSerializersTest extends ForyTestBase { Assert.assertTrue(bytes1.length > bytes2.length); assertEquals(fory.deserialize(bytes2), data); fory.getGenerics().popGenericType(); - Assert.assertThrows(RuntimeException.class, () -> fory.deserialize(bytes2)); + assertThrowsCause(RuntimeException.class, () -> fory.deserialize(bytes2)); } @Test(dataProvider = "referenceTrackingConfig") @@ -191,7 +191,7 @@ public class CollectionSerializersTest extends ForyTestBase { byte[] bytes2 = fory.serialize(data); Assert.assertTrue(bytes1.length > bytes2.length); fory.getGenerics().popGenericType(); - Assert.assertThrows(RuntimeException.class, () -> fory.deserialize(bytes2)); + assertThrowsCause(RuntimeException.class, () -> fory.deserialize(bytes2)); } @Test(dataProvider = "foryCopyConfig") diff --git a/java/fory-core/src/test/java/org/apache/fory/serializer/collection/MapSerializersTest.java b/java/fory-core/src/test/java/org/apache/fory/serializer/collection/MapSerializersTest.java index a9f078c11..b19b76e26 100644 --- a/java/fory-core/src/test/java/org/apache/fory/serializer/collection/MapSerializersTest.java +++ b/java/fory-core/src/test/java/org/apache/fory/serializer/collection/MapSerializersTest.java @@ -97,7 +97,7 @@ public class MapSerializersTest extends ForyTestBase { byte[] bytes2 = fory.serialize(data); Assert.assertTrue(bytes1.length > bytes2.length); fory.getGenerics().popGenericType(); - Assert.assertThrows(RuntimeException.class, () -> fory.deserialize(bytes2)); + assertThrowsCause(RuntimeException.class, () -> fory.deserialize(bytes2)); // testSortedMap Map<String, Integer> treeMap = new TreeMap<>(ImmutableMap.of("a", 1, "b", 2)); @@ -107,7 +107,7 @@ public class MapSerializersTest extends ForyTestBase { byte[] sortMapBytes2 = fory.serialize(treeMap); Assert.assertTrue(sortMapBytes1.length > sortMapBytes2.length); fory.getGenerics().popGenericType(); - Assert.assertThrows(RuntimeException.class, () -> fory.deserialize(sortMapBytes2)); + assertThrowsCause(RuntimeException.class, () -> fory.deserialize(sortMapBytes2)); // testTreeMap TreeMap<String, String> map = @@ -260,7 +260,7 @@ public class MapSerializersTest extends ForyTestBase { byte[] bytes2 = fory.serialize(data); Assert.assertTrue(bytes1.length > bytes2.length); fory.getGenerics().popGenericType(); - Assert.assertThrows(RuntimeException.class, () -> fory.deserialize(bytes2)); + assertThrowsCause(RuntimeException.class, () -> fory.deserialize(bytes2)); } @Test(dataProvider = "referenceTrackingConfig") @@ -278,7 +278,7 @@ public class MapSerializersTest extends ForyTestBase { byte[] bytes2 = fory.serialize(data); Assert.assertTrue(bytes1.length > bytes2.length); fory.getGenerics().popGenericType(); - Assert.assertThrows(RuntimeException.class, () -> fory.deserialize(bytes2)); + assertThrowsCause(RuntimeException.class, () -> fory.deserialize(bytes2)); } @Test(dataProvider = "foryCopyConfig") --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
