Skip to content

Conversation

@SylvainJuge
Copy link
Contributor

Part of #13031

@SylvainJuge SylvainJuge self-assigned this Oct 24, 2025
@SylvainJuge SylvainJuge requested a review from a team as a code owner October 24, 2025 12:30
@SylvainJuge SylvainJuge marked this pull request as draft October 24, 2025 12:51
Comment on lines +88 to +90
assertThat(SomeClass.isInstrumented())
.describedAs("method return value should be preserved when exception is thrown in advice")
.isFalse();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[for reviewer] in the past the method return value could be modified when advice throws an unexpected exception, now that the return value is modified through the advice method return value this is no longer possible, and thus we always get the original method return value here (and this is very probably a good thing).

Comment on lines -13 to 15
@Advice.OnMethodExit(suppress = Throwable.class)
public static void throwAnException(@Advice.Return(readOnly = false) boolean returnVal) {
returnVal = true;
public static boolean throwAnException() {
throw new IllegalStateException("Test Exception");
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[for reviewer] with the original syntax, the method return value is altered even when the advice throws an exception, this is no longer the case with this syntax.

@SylvainJuge SylvainJuge marked this pull request as ready for review October 24, 2025 14:28
@SylvainJuge SylvainJuge marked this pull request as draft October 24, 2025 16:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant