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

Observation#wrapChecked(CheckedCallable<T,E>) should not throw E itself #5105

Open
nobeh opened this issue May 15, 2024 · 1 comment
Open
Labels
enhancement A general enhancement
Milestone

Comments

@nobeh
Copy link

nobeh commented May 15, 2024

Describe the bug

io.micrometer.observation.Observation#wrapChecked(Observation.CheckedCallable<T,E>) is as follows:

default <T, E extends Throwable> CheckedCallable<T, E> 
    wrapChecked(CheckedCallable<T, E> checkedCallable) throws E {
        return () -> observeChecked(checkedCallable);
}

It wraps around a Callable that throws E. The return of this method is another Callable that would delegate to the provided code and also makes an observation.

If we rewrite the method as:

default <T, E extends Throwable> CheckedCallable<T, E> 
    wrapChecked(CheckedCallable<T, E> checkedCallable) throws E {
        var observedCallable = () -> observeChecked(checkedCallable);
        return observedCallable;
}

Then there is no need that the method wrapChecked should also throw E. The wrapping logic should not encounter any exception. I think this might have been a mistake during design or compilation.

@nobeh nobeh changed the title Observation#wrapChecked(io.micrometer.observation.Observation.CheckedCallable<T,E>) should not throw E itself Observation#wrapChecked(CheckedCallable<T,E>) should not throw E itself May 15, 2024
@shakuzen
Copy link
Member

shakuzen commented May 16, 2024

Thanks for the report. That does seem like a mistake. The problem is I think while it would be binary compatible to remove the checked exception (throws), I don't think it would be source compatible.

@shakuzen shakuzen added enhancement A general enhancement and removed waiting-for-triage labels May 16, 2024
@shakuzen shakuzen added this to the 2.0.0 backlog milestone May 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

2 participants