Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TraceSessionRepositoryAspect#callMethodOnWrappedObject does not handle InvocationTargetException #2246

Closed
robert-gdv opened this issue Jan 11, 2023 · 4 comments · Fixed by #2264
Labels
bug status: ideal-for-contribution An issue that a contributor can help us with
Milestone

Comments

@robert-gdv
Copy link

Describe the bug
I am using spring-cloud-sleuth-instrumentation-3.1.5.jar in combination with spring-aop-5.3.24.jar

The method callMethodOnWrappedObject(ProceedingJoinPoint pjp, T target) invokes a method to access a sessionRepository.
But it doesn't handle the case properly, when the invoked method throws a RuntimeException or another declared Exception.

The result of this bug is, that a org.springframework.web.reactive.handler.WebFluxResponseStatusExceptionHandler and perhaps other places also see a java.lang.reflect.UndeclaredThrowableException instead of the actually thrown exception.
The UndeclaredThrowableException is created in https://github.com/spring-projects/spring-framework/blob/46875c91ced9387a598c3ca7eba3ed23fdb4fc63/spring-aop/src/main/java/org/springframework/aop/framework/CglibAopProxy.java#L769
because the called sessionRepository method doesn't declare to throw an InvocationTargetException (which is correct).

Current implementation:

Suggested implementation:

try {
  return method.invoke(target, pjp.getArgs());
} catch(InvocationTargetException ex) {
  throw ex.getCause();
}

similar to https://github.com/spring-projects/spring-framework/blob/46875c91ced9387a598c3ca7eba3ed23fdb4fc63/spring-aop/src/main/java/org/springframework/aop/aspectj/AbstractAspectJAdvice.java#L640
or you use org.springframework.util.ReflectionUtils.invokeMethod.

I found a similar issue #1092. Maybe there are more like this.

@jonatan-ivanov
Copy link
Member

Are you defining the version numbers manually or using the BOM?
If manually, could you please try it with the BOM and let us know it it worked?

@robert-gdv
Copy link
Author

@jonatan-ivanov I am using the BOM.
The relevant parts in my pom.xml are:

    <parent>
        <groupId>org.springframework.boot</groupId>
        <artifactId>spring-boot-starter-parent</artifactId>
        <version>2.7.7</version>
        <relativePath />
    </parent>
    <dependencyManagement>
        <dependencies>
            <dependency>
                <groupId>org.springframework.cloud</groupId>
                <artifactId>spring-cloud-dependencies</artifactId>
                <version>2021.0.5</version>
                <type>pom</type>
                <scope>import</scope>
            </dependency>
         </dependencies>
    </dependencyManagement>
    <dependencies>
        <dependency>
            <groupId>org.springframework.cloud</groupId>
            <artifactId>spring-cloud-starter-gateway</artifactId>
        </dependency>
        <dependency>
            <groupId>org.springframework.session</groupId>
            <artifactId>spring-session-core</artifactId>
        </dependency>
        <dependency>
            <groupId>org.springframework</groupId>
            <artifactId>spring-webflux</artifactId>
        </dependency>
        <dependency>
            <groupId>org.springframework.cloud</groupId>
            <artifactId>spring-cloud-starter-sleuth</artifactId>
        </dependency>
        <dependency>
            <groupId>io.projectreactor.netty</groupId>
            <artifactId>reactor-netty-http-brave</artifactId>
        </dependency>
     </dependencies>

@marcingrzejszczak
Copy link
Contributor

Are you willing to file a PR to fix this?

@robert-gdv
Copy link
Author

I am currently not allowed to sign the Contributor License Agreement. But this should be doable by anyone with a litte java skillset. The code is already provided in the bug description and not even my own, because I copied it from another spring project.

@marcingrzejszczak marcingrzejszczak added the status: ideal-for-contribution An issue that a contributor can help us with label Feb 17, 2023
thermoweb added a commit to thermoweb/spring-cloud-sleuth that referenced this issue Feb 17, 2023
@marcingrzejszczak marcingrzejszczak added this to the 3.1.7 milestone Feb 17, 2023
@github-project-automation github-project-automation bot moved this to Done in 2021.0.6 Feb 17, 2023
marcingrzejszczak added a commit that referenced this issue Feb 17, 2023
* Fixes gh-2246 : handle InvocationTargetException properly

* Polished the solution + added tests

---------

Co-authored-by: romain.chauveau <romain.chauveau@praxedo.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug status: ideal-for-contribution An issue that a contributor can help us with
Projects
No open projects
Status: Done
Development

Successfully merging a pull request may close this issue.

4 participants