-
Notifications
You must be signed in to change notification settings - Fork 3k
Allow forcing exception unwrapping even when parent type mappers exist #50730
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
Allow forcing exception unwrapping even when parent type mappers exist #50730
Conversation
This comment has been minimized.
This comment has been minimized.
56ef700 to
a84b26d
Compare
This comment has been minimized.
This comment has been minimized.
|
🙈 The PR is closed and the preview is expired. |
This comment has been minimized.
This comment has been minimized.
a84b26d to
d10b3c1
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
d10b3c1 to
d2d9f1f
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
d2d9f1f to
8b4a024
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
8b4a024 to
543975d
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
543975d to
47682be
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
| () -> searchMapperForExceptionsToUnwrap(clazz, mappers, throwable, ExceptionUnwrapStrategy.ALWAYS), | ||
| // If the exception type is directly mapped, ignore the unwrapping. | ||
| () -> buildMapperEntryIfExists(clazz, mappers, throwable), | ||
| // Check if the exception should be unwrapped if no exact match (if strategy=UNWRAP_IF_NO_EXACT_MATCH). | ||
| () -> searchMapperForExceptionsToUnwrap(clazz, mappers, throwable, | ||
| ExceptionUnwrapStrategy.UNWRAP_IF_NO_EXACT_MATCH), | ||
| // Walk up the class hierarchy looking for a mapper for the type | ||
| () -> searchMapperInClassHierarchy(clazz, mappers, throwable), | ||
| // If no mapper found and exception is marked for unwrapping (if strategy=UNWRAP_IF_NO_MATCH), unwrap it | ||
| () -> searchMapperForExceptionsToUnwrap(clazz, mappers, throwable, ExceptionUnwrapStrategy.UNWRAP_IF_NO_MATCH)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We try hard to avoid lambdas in code that executes at runtime. Can you please update the code to remove them?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Implemented, waiting for a green build 😄
47682be to
bd474ea
Compare
geoand
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
This comment has been minimized.
This comment has been minimized.
bd474ea to
434ca0a
Compare
Status for workflow
|
Status for workflow
|
Adds support for the strategy attribute in @UnwrapException annotation to control exception unwrapping behaviour.
This makes tests in the reproducer pass with following annotation:
https://github.com/user-attachments/files/20574949/quarkus-48197-reproducer.zip
Main Changes:
Added strategy boolean parameter to @UnwrapException annotation (default: UNWRAP_IF_NO_MATCH) and propagated it.
Updated RuntimeExceptionMapper exception mapping strategy to handle it:
Reworked UnwrappedExceptionTest to add more cases and more explicit names, and achieve 100% coverage on new code in RuntimeExceptionMapper.
Closes: UnwrapException ineffective when a subclass of the declared exception is mapped via ServerExceptionMapper #48197