Skip to content

Package-private @GrpcAdvice classes fail to handle exceptions #442

Closed

Description

@CCCLL

On current main (31de8166a178564914eab329cc5ca0d4871b6a62, 1.1.2-SNAPSHOT), an exception handler in a package-private @GrpcAdvice class is discovered successfully but fails when invoked from GrpcAdviceExceptionHandler.

For example, an application can declare the following class outside org.springframework.grpc.server.advice:

package example;

import org.springframework.grpc.server.advice.GrpcAdvice;
import org.springframework.grpc.server.advice.GrpcExceptionHandler;

import io.grpc.Status;

@GrpcAdvice
class ApplicationAdvice {

    @GrpcExceptionHandler
    public Status handleIllegalArgument(IllegalArgumentException exception) {
        return Status.INVALID_ARGUMENT.withDescription(exception.getMessage());
    }

}

With this bean registered in an AnnotationConfigApplicationContext alongside GrpcAdviceDiscoverer, GrpcExceptionHandlerMethodResolver, and GrpcAdviceExceptionHandler, handling new IllegalArgumentException("bad argument") should return INVALID_ARGUMENT with description bad argument.

Instead, it returns INTERNAL with description There was a server error trying to handle an exception. The underlying cause is IllegalAccessException: the framework cannot access a public method whose declaring class is not public and is in another package.

Changing only the advice class to public makes the same test pass. The existing package-private advice fixture is in the framework implementation's package, so it does not exercise this access boundary.

The failure occurs at mappedMethod.invoke(...) in GrpcAdviceExceptionHandler.invokeMappedMethodSafely. Making the method accessible before invoking it appears sufficient to preserve the current exception matching and return-value handling. Neither annotation currently documents a public-class requirement.

The accompanying test patch adds a public and a package-private advice fixture in another package. With Java 25, the existing six GrpcAdviceExceptionHandlerTests tests and the public-class control pass; the package-private-class test fails with expected: INVALID_ARGUMENT, but was: INTERNAL.

The reproduction exercises the real Spring context and exception handler, without requiring a running gRPC server. It can be run with:

./mvnw -pl spring-grpc-core -am \
  -Dtest=GrpcAdviceExceptionHandlerTests \
  -Dsurefire.failIfNoSpecifiedTests=false test
Regression test patch

Save the following diff as advice-visibility.patch and apply it with git apply advice-visibility.patch at the repository root before running the command above.

diff --git a/spring-grpc-core/src/test/java/org/springframework/grpc/server/advice/GrpcAdviceExceptionHandlerTests.java b/spring-grpc-core/src/test/java/org/springframework/grpc/server/advice/GrpcAdviceExceptionHandlerTests.java
index 1360b2f..122213f 100644
--- a/spring-grpc-core/src/test/java/org/springframework/grpc/server/advice/GrpcAdviceExceptionHandlerTests.java
+++ b/spring-grpc-core/src/test/java/org/springframework/grpc/server/advice/GrpcAdviceExceptionHandlerTests.java
@@ -26,6 +26,7 @@ import org.springframework.context.ApplicationContext;
 import org.springframework.context.annotation.AnnotationConfigApplicationContext;
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
+import org.springframework.grpc.server.advice.support.AdviceVisibilitySamples;
 
 import io.grpc.Metadata;
 import io.grpc.Status;
@@ -36,9 +37,32 @@ import io.grpc.StatusRuntimeException;
  * Tests for {@link GrpcAdviceExceptionHandler}.
  *
  * @author Oleksandr Shevchenko
+ * @author Jialin Chen
  */
 class GrpcAdviceExceptionHandlerTests {
 
+	@Test
+	void handlesExceptionInPublicAdviceFromAnotherPackage() {
+		assertAdviceHandlesException(AdviceVisibilitySamples.PublicAdvice.class);
+	}
+
+	@Test
+	void handlesExceptionInPackagePrivateAdviceFromAnotherPackage() {
+		assertAdviceHandlesException(AdviceVisibilitySamples.packagePrivateAdviceType());
+	}
+
+	private void assertAdviceHandlesException(Class<?> adviceType) {
+		try (AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext(adviceType,
+				GrpcAdviceDiscoverer.class, GrpcExceptionHandlerMethodResolver.class,
+				GrpcAdviceExceptionHandler.class)) {
+			GrpcAdviceExceptionHandler handler = context.getBean(GrpcAdviceExceptionHandler.class);
+			StatusException result = handler.handleException(new IllegalArgumentException("bad argument"));
+			assertThat(result).isNotNull();
+			assertThat(result.getStatus().getCode()).isEqualTo(Status.Code.INVALID_ARGUMENT);
+			assertThat(result.getStatus().getDescription()).isEqualTo("bad argument");
+		}
+	}
+
 	@Test
 	void handlesExceptionWithMappedHandler() {
 		try (AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext(TestConfig.class)) {
diff --git a/spring-grpc-core/src/test/java/org/springframework/grpc/server/advice/support/AdviceVisibilitySamples.java b/spring-grpc-core/src/test/java/org/springframework/grpc/server/advice/support/AdviceVisibilitySamples.java
new file mode 100644
index 0000000..4925bc7
--- /dev/null
+++ b/spring-grpc-core/src/test/java/org/springframework/grpc/server/advice/support/AdviceVisibilitySamples.java
@@ -0,0 +1,58 @@
+/*
+ * Copyright 2026-present the original author or authors.
+ *
+ * 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
+ *
+ *      https://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.springframework.grpc.server.advice.support;
+
+import org.springframework.grpc.server.advice.GrpcAdvice;
+import org.springframework.grpc.server.advice.GrpcExceptionHandler;
+
+import io.grpc.Status;
+
+/**
+ * Advice fixtures in a different package from the exception handler implementation.
+ *
+ * @author Jialin Chen
+ */
+public final class AdviceVisibilitySamples {
+
+	private AdviceVisibilitySamples() {
+	}
+
+	public static Class<?> packagePrivateAdviceType() {
+		return PackagePrivateAdvice.class;
+	}
+
+	@GrpcAdvice
+	public static class PublicAdvice {
+
+		@GrpcExceptionHandler
+		public Status handleIllegalArgument(IllegalArgumentException exception) {
+			return Status.INVALID_ARGUMENT.withDescription(exception.getMessage());
+		}
+
+	}
+
+	@GrpcAdvice
+	static class PackagePrivateAdvice {
+
+		@GrpcExceptionHandler
+		public Status handleIllegalArgument(IllegalArgumentException exception) {
+			return Status.INVALID_ARGUMENT.withDescription(exception.getMessage());
+		}
+
+	}
+
+}

If you agree that this should be fixed, I’d be happy to work on a fix and submit a PR.

Activity

  1. dsyer commented on Oct 12, 2026

    @dsyer
    Member

    Happy to look at the patch.

  2. dhanush080607 commented on Oct 14, 2026

    @dhanush080607

    Thanks! I'll work on the fix, add regression coverage for package-private @GrpcAdvice classes in a different package, and run the relevant tests before submitting a PR.

  3. CCCLL commented on Oct 15, 2026

    @CCCLL
    ContributorAuthor

    Thanks for offering to help, @dhanush080607. I have now submitted the fix and regression tests in #443. To avoid duplicate work, please hold off on a separate implementation. Feedback on the PR is welcome.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions