This is an automated email from the ASF dual-hosted git repository.
yuqi1129 pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/main by this push:
new 7239078b10 [#13401] fix(core): unwrap InvocationTargetException in
OperationsProxy.invoke (#13402)
7239078b10 is described below
commit 7239078b1072742984b7b0411022e2fd4d4992c1
Author: YangJie <[email protected]>
AuthorDate: Tue Sep 22 05:53:57 2026 -0400
[#13401] fix(core): unwrap InvocationTargetException in
OperationsProxy.invoke (#13402)
### What changes were proposed in this pull request?
`OperationsProxy.invoke` now rethrows the target exception from the
`InvocationTargetException` that reflection produces, matching the
standard proxy-handler idiom already used in `DynMethods`. Callers see
the operation's own exception, the same one a direct call would throw.
### Why are the changes needed?
When a catalog supplies a `ProxyPlugin`, the reflective wrapper escaped
instead of the real exception, so exception-type dispatch such as the
REST layer's status mapping matched none of its `instanceof` branches
and returned a generic 500 rather than the correct 404 or 409.
Fix: #13401
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
Added `TestOperationsProxy.testExceptionsAreUnwrapped` (a typed
exception thrown through a proxied operation reaches the caller
unwrapped) and `testNormalInvocationPassesThrough` (a normal call is
unaffected). The unwrap assertion fails against the pre-fix code.
---
.../gravitino/connector/OperationsProxy.java | 16 +++-
.../gravitino/connector/TestOperationsProxy.java | 97 ++++++++++++++++++++++
2 files changed, 109 insertions(+), 4 deletions(-)
diff --git
a/core/src/main/java/org/apache/gravitino/connector/OperationsProxy.java
b/core/src/main/java/org/apache/gravitino/connector/OperationsProxy.java
index 14ba65ca06..d19054063f 100644
--- a/core/src/main/java/org/apache/gravitino/connector/OperationsProxy.java
+++ b/core/src/main/java/org/apache/gravitino/connector/OperationsProxy.java
@@ -19,6 +19,7 @@
package org.apache.gravitino.connector;
import java.lang.reflect.InvocationHandler;
+import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
import java.lang.reflect.Proxy;
import java.util.Collections;
@@ -54,9 +55,16 @@ public class OperationsProxy<T> implements InvocationHandler
{
@Override
public Object invoke(Object proxy, Method method, Object[] args) throws
Throwable {
- return plugin.doAs(
- PrincipalUtils.getCurrentPrincipal(),
- () -> method.invoke(ops, args),
- Collections.emptyMap());
+ try {
+ return plugin.doAs(
+ PrincipalUtils.getCurrentPrincipal(),
+ () -> method.invoke(ops, args),
+ Collections.emptyMap());
+ } catch (InvocationTargetException e) {
+ // Surface the operation's own exception instead of the reflective
wrapper, so
+ // callers dispatching on exception type (e.g. REST status mapping) keep
working.
+ Throwable cause = e.getCause();
+ throw cause != null ? cause : e;
+ }
}
}
diff --git
a/core/src/test/java/org/apache/gravitino/connector/TestOperationsProxy.java
b/core/src/test/java/org/apache/gravitino/connector/TestOperationsProxy.java
new file mode 100644
index 0000000000..53a968614f
--- /dev/null
+++ b/core/src/test/java/org/apache/gravitino/connector/TestOperationsProxy.java
@@ -0,0 +1,97 @@
+/*
+ * 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.gravitino.connector;
+
+import java.security.Principal;
+import java.util.Map;
+import java.util.concurrent.atomic.AtomicInteger;
+import org.apache.gravitino.NameIdentifier;
+import org.apache.gravitino.exceptions.NoSuchTableException;
+import org.apache.gravitino.utils.Executable;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+
+/** Tests for {@link OperationsProxy}. */
+public class TestOperationsProxy {
+
+ private static class PassthroughProxyPlugin implements ProxyPlugin {
+ @Override
+ public Object doAs(
+ Principal principal, Executable<Object, Exception> action, Map<String,
String> properties)
+ throws Throwable {
+ return action.execute();
+ }
+
+ @Override
+ public void bindCatalogOperation(CatalogOperations ops) {}
+ }
+
+ @Test
+ public void testExceptionsAreUnwrapped() {
+ CatalogOperations ops =
+ new CatalogOperations() {
+ @Override
+ public void initialize(
+ Map<String, String> config,
+ CatalogInfo catalogInfo,
+ HasPropertyMetadata hasPropertyMetadata) {}
+
+ @Override
+ public void testConnection(NameIdentifier catalogIdent) throws
Exception {
+ throw new NoSuchTableException("table does not exist");
+ }
+
+ @Override
+ public void close() {}
+ };
+
+ CatalogOperations proxy = OperationsProxy.createProxy(ops, new
PassthroughProxyPlugin());
+
+ // The caller must see the operation's own exception, not the reflective
+ // InvocationTargetException wrapper, so exception-type dispatch keeps
working.
+ Assertions.assertThrows(
+ NoSuchTableException.class, () ->
proxy.testConnection(NameIdentifier.of("catalog")));
+ }
+
+ @Test
+ public void testNormalInvocationPassesThrough() throws Exception {
+ AtomicInteger calls = new AtomicInteger();
+ CatalogOperations ops =
+ new CatalogOperations() {
+ @Override
+ public void initialize(
+ Map<String, String> config,
+ CatalogInfo catalogInfo,
+ HasPropertyMetadata hasPropertyMetadata) {}
+
+ @Override
+ public void testConnection(NameIdentifier catalogIdent) {
+ calls.incrementAndGet();
+ }
+
+ @Override
+ public void close() {}
+ };
+
+ CatalogOperations proxy = OperationsProxy.createProxy(ops, new
PassthroughProxyPlugin());
+ proxy.testConnection(NameIdentifier.of("catalog"));
+
+ Assertions.assertEquals(1, calls.get());
+ }
+}
