Comments (5)
Before the change introduced in #32359, the following would happen:
- the exception thrown by the controller would be handled first by the
DispatcherHandler
by calling the exception handler here. - the
@ExceptionHandler
rethrows another exception - then the
RequestMappingHandlerAdapter
would handle this rethrown exception
I think this behavior is invalid and unintended for the following reasons:
- the same application in Spring MVC does not behave like this, rethrown exceptions are not processed by other handlers
- this behavior would only work once - rethrowing the exception another time would not work
- this has been the intended and documented behavior for
@ExceptionHandler
methods for a very long time, see https://docs.spring.io/spring-framework/reference/web/webmvc/mvc-controller/ann-exceptionhandler.html
Still, we can consider rolling back partially this change as this is disrupting applications in a maintenance release. But I do think that keeping this change around for Spring Framework 6.2 is a good choice.
from spring-framework.
@mbanaszkiewicz-vonage @beytularedzheb Thanks for raising this issue. I discussed this with @rstoyanchev and we confirmed that the original behavior was invalid in the first place and that exception handlers are supposed to fully handle the exception or re-throw the original exception.
I initially marked this as a bug for 6.1.x as I thought the the handling of exceptions in different phases was maybe a mistake, but it turns out this is by design in WebFlux and that we may consolidate things in the future.
In the meantime, can you let us know whether returning a ResponseEntity
would work for your application? We would like to know why you chose this setup in the first place. Thanks!
from spring-framework.
Also see spring-projects/spring-boot#40148 (comment) for another report of this.
from spring-framework.
5f601ce handle exceptions from @ExceptionHandler regardless of whether
thrown immediately or via Publisher.
private static Mono<HandlerResult> handleExceptionHandlerFailure(
ServerWebExchange exchange, Throwable exception, Throwable invocationEx,
ArrayList<Throwable> exceptions, InvocableHandlerMethod invocable) {
if (disconnectedClientHelper.checkAndLogClientDisconnectedException(invocationEx)) {
return Mono.empty();
}
// Any other than the original exception (or a cause) is unintended here,
// probably an accident (e.g. failed assertion or the like).
if (!exceptions.contains(invocationEx) && logger.isWarnEnabled()) {
// ☝️☝️ if exception is 500 and invocationEx from ExceptionHandler is 400, intend to respect 500 here?
logger.warn(exchange.getLogPrefix() + "Failure in @ExceptionHandler " + invocable, invocationEx);
}
return Mono.error(exception); // ignore invocationEx and return origin exception!
}
Hmm maybe it's intended behavior from this 5f601ce commit?
// Maybe we can fix it like this?
if (invocationEx != null && invocationEx instanceof ResponseStatusException) {
return Mono.error(invocationEx);
}
return Mono.error(exception);
Maybe we can fix like above code, let's wait maintainer's opinion~!
If my suggestion makes sense, I'm ready to open fix PR :) thank you!
from spring-framework.
Hi @bclozel,
Thank you for checking it.
It was just an easy & quick way of changing the status code and getting standard error response with already populated fields, like path, request id, etc. (DefaultErrorAttributes).
Actually, as you mentioned ResponseEntity, I realized that I can still use ResponseStatusException and achieve the same effect.
Instead of throwing an exception directly in the exception handler method, I can return it in a Mono:
@RestControllerAdvice
internal class RestControllerExceptionHandler {
@ExceptionHandler(ConstraintViolationException::class)
fun exceptionHandler(e: ConstraintViolationException): Mono<Throwable> {
return Mono.error(ResponseStatusException(HttpStatus.BAD_REQUEST, e.message, e))
}
}
This should also work for the case you shared above: spring-projects/spring-boot#40148 (comment)
I am closing my ticket.
And thank you again!
from spring-framework.
Related Issues (20)
- Observation in ServerHttpObservationFilter is never stopped for asynchronous requests HOT 1
- ContentCachingRequestWrapper may allocate too much memory
- Bean override with REPLACE_OR_CREATE_DEFINITION and byType lookup fails if no match is found
- Quartz does not understand Cron expression when dayOfweek is 2-6 and dayOfMonth is * HOT 3
- RestClient does not work when sending an object HOT 1
- Inconsistent character handling between MockMvc created from a context or standalone HOT 9
- Exception that prevents component scan with REGISTER_BEAN conditions should provide the affected configuration class
- Upgrade to Micrometer 1.13.1
- Upgrade to Micrometer 1.12.7
- Ambiguity in doc HOT 2
- ActiveMQ messages for a group are not getting consumed and keeps piling HOT 2
- DataClassRowMapper cannot find record construcor during native run HOT 1
- OverrideMetadata equals/hashCode should rely on getClass() HOT 1
- Upgrade to Reactor 2024.0.0-M3
- Upgrade to Reactor 2023.0.7
- Upgrade to Reactor 2022.0.20 HOT 1
- Behaviour change in ScheduledAnnotationBeanPostProcessor: canceling scheduled tasks on ContextClosedEvent v6.0 -> v6.1 HOT 4
- Upgrade to Reactor 2020.0.45 HOT 1
- Support Protobuf 4.x
- TransactionalEventListener called with open transaction HOT 2
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from spring-framework.