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

TimedAspect advice return value should be marked Nullable #3041

Closed
tux-mind opened this issue Feb 26, 2022 · 2 comments
Closed

TimedAspect advice return value should be marked Nullable #3041

tux-mind opened this issue Feb 26, 2022 · 2 comments
Labels
bug A general bug module: micrometer-core An issue that is related to our core module
Milestone

Comments

@tux-mind
Copy link

Describe the bug
io.micrometer.core.aop.TimedAspect is annotated with @nonNullApi, however its public Object timedMethod(ProceedingJoinPoint pjp) throws Throwable returns null objects when the wrapped function does so ( or returns void ).

Environment
micrometer-core:1.3.0

To Reproduce
When using this class from Kotlin, the compiler expects that a non-null object is returned and fails at runtime when a null is returned.
Or, changing POV, it is impossible to override the function and return Kotlin's nullable object Any? as the compiler treat the signature as non-nullable.

Expected behavior
The returned value shall be annotated as @Nullable as it is null in some circumstances.

Additional context
This issue is still present on your main branch as of today (

public Object timedMethod(ProceedingJoinPoint pjp) throws Throwable {
).

@tux-mind tux-mind added the bug A general bug label Feb 26, 2022
@shakuzen shakuzen added the module: micrometer-core An issue that is related to our core module label Mar 4, 2022
@shakuzen
Copy link
Member

shakuzen commented Mar 4, 2022

This sounds like something we can and should fix, but I'm curious, why would you be calling that method directly in your code? Or is this a problem even when not directly calling the method in your code?

@shakuzen shakuzen added this to the 1.7.x milestone Mar 4, 2022
@tux-mind
Copy link
Author

tux-mind commented Mar 4, 2022

I wanted to time the execution time of Monos ( form project reactor a.k.a. RxJava ).
Monos are assembled when the timed method is called, but executed later. Therefore, my solution was to override the method timedMethod and inspect the returned type of the wrapped method.
When the timed method is returning a Mono, return a Mono.fromCallable { setupTimers... }.then(pjp.preceed() as Mono<?>).doFinally { finishTimers } ( there are still cases to cover, like cancellation, but this works for the moment being ).

I was able to do so in Java, but not in Kotlin, due to the reported issue.

@shakuzen shakuzen modified the milestones: 1.7.x, 1.7.11 Apr 14, 2022
@shakuzen shakuzen changed the title overused @NonNullApi in TimedAspect TimedAspect advice return value should be marked Nullable Apr 14, 2022
shakuzen added a commit that referenced this issue Apr 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A general bug module: micrometer-core An issue that is related to our core module
Projects
None yet
Development

No branches or pull requests

2 participants