Make CodecException handling consistent
This commit makes CodecException handling consistent between functional and annotation-based APIs. It now returns by default 4xx status code for decoding error and 5xx for encoding error + print the error reason in logs without the full stack trace in both variants. Issue: SPR-15355
This commit is contained in:
parent
15b5dd9f12
commit
d098a4b96b
|
@ -21,6 +21,9 @@ import java.util.HashMap;
|
|||
import java.util.List;
|
||||
import java.util.Map;
|
||||
|
||||
import org.springframework.core.codec.CodecException;
|
||||
import org.springframework.http.HttpStatus;
|
||||
import org.springframework.web.server.ResponseStatusException;
|
||||
import reactor.core.publisher.Flux;
|
||||
import reactor.core.publisher.Mono;
|
||||
|
||||
|
@ -85,7 +88,9 @@ public class DecoderHttpMessageReader<T> implements HttpMessageReader<T> {
|
|||
Map<String, Object> hints) {
|
||||
|
||||
MediaType contentType = getContentType(message);
|
||||
return this.decoder.decode(message.getBody(), elementType, contentType, hints);
|
||||
return this.decoder
|
||||
.decode(message.getBody(), elementType, contentType, hints)
|
||||
.mapError(this::mapError);
|
||||
}
|
||||
|
||||
@Override
|
||||
|
@ -93,7 +98,9 @@ public class DecoderHttpMessageReader<T> implements HttpMessageReader<T> {
|
|||
Map<String, Object> hints) {
|
||||
|
||||
MediaType contentType = getContentType(message);
|
||||
return this.decoder.decodeToMono(message.getBody(), elementType, contentType, hints);
|
||||
return this.decoder
|
||||
.decodeToMono(message.getBody(), elementType, contentType, hints)
|
||||
.mapError(this::mapError);
|
||||
}
|
||||
|
||||
private MediaType getContentType(HttpMessage inputMessage) {
|
||||
|
@ -101,6 +108,16 @@ public class DecoderHttpMessageReader<T> implements HttpMessageReader<T> {
|
|||
return (contentType != null ? contentType : MediaType.APPLICATION_OCTET_STREAM);
|
||||
}
|
||||
|
||||
private Throwable mapError(Throwable ex) {
|
||||
if (ex instanceof ResponseStatusException) {
|
||||
return ex;
|
||||
}
|
||||
else if (ex instanceof CodecException) {
|
||||
return new ResponseStatusException(HttpStatus.BAD_REQUEST, "Failed to decode HTTP message", ex);
|
||||
}
|
||||
return new ResponseStatusException(HttpStatus.INTERNAL_SERVER_ERROR, "Failed to decode HTTP message", ex);
|
||||
}
|
||||
|
||||
|
||||
// Server-side only...
|
||||
|
||||
|
|
|
@ -22,6 +22,8 @@ import java.util.List;
|
|||
import java.util.Map;
|
||||
|
||||
import org.reactivestreams.Publisher;
|
||||
import org.springframework.http.HttpStatus;
|
||||
import org.springframework.web.server.ResponseStatusException;
|
||||
import reactor.core.publisher.Flux;
|
||||
import reactor.core.publisher.Mono;
|
||||
|
||||
|
@ -95,8 +97,9 @@ public class EncoderHttpMessageWriter<T> implements HttpMessageWriter<T> {
|
|||
|
||||
MediaType contentType = updateContentType(message, mediaType);
|
||||
|
||||
Flux<DataBuffer> body = this.encoder.encode(
|
||||
inputStream, message.bufferFactory(), elementType, contentType, hints);
|
||||
Flux<DataBuffer> body = this.encoder
|
||||
.encode(inputStream, message.bufferFactory(), elementType, contentType, hints)
|
||||
.mapError(this::mapError);
|
||||
|
||||
return isStreamingMediaType(contentType) ?
|
||||
message.writeAndFlushWith(body.map(Flux::just)) :
|
||||
|
@ -135,6 +138,13 @@ public class EncoderHttpMessageWriter<T> implements HttpMessageWriter<T> {
|
|||
.anyMatch(contentType::isCompatibleWith);
|
||||
}
|
||||
|
||||
private Throwable mapError(Throwable ex) {
|
||||
if (ex instanceof ResponseStatusException) {
|
||||
return ex;
|
||||
}
|
||||
return new ResponseStatusException(HttpStatus.INTERNAL_SERVER_ERROR, "Failed to encode HTTP message", ex);
|
||||
}
|
||||
|
||||
|
||||
// Server side only...
|
||||
|
||||
|
|
|
@ -16,6 +16,8 @@
|
|||
|
||||
package org.springframework.web.server.handler;
|
||||
|
||||
import org.apache.commons.logging.Log;
|
||||
import org.apache.commons.logging.LogFactory;
|
||||
import reactor.core.publisher.Mono;
|
||||
|
||||
import org.springframework.web.server.ResponseStatusException;
|
||||
|
@ -26,14 +28,20 @@ import org.springframework.web.server.WebExceptionHandler;
|
|||
* Handle {@link ResponseStatusException} by setting the response status.
|
||||
*
|
||||
* @author Rossen Stoyanchev
|
||||
* @author Sebastien Deleuze
|
||||
* @since 5.0
|
||||
*/
|
||||
public class ResponseStatusExceptionHandler implements WebExceptionHandler {
|
||||
|
||||
private static final Log logger = LogFactory.getLog(ResponseStatusExceptionHandler.class);
|
||||
|
||||
@Override
|
||||
public Mono<Void> handle(ServerWebExchange exchange, Throwable ex) {
|
||||
if (ex instanceof ResponseStatusException) {
|
||||
exchange.getResponse().setStatusCode(((ResponseStatusException) ex).getStatus());
|
||||
if (ex.getMessage() != null) {
|
||||
logger.error(ex.getMessage());
|
||||
}
|
||||
return Mono.empty();
|
||||
}
|
||||
return Mono.error(ex);
|
||||
|
|
|
@ -237,6 +237,9 @@ public abstract class RouterFunctions {
|
|||
.otherwise(ResponseStatusException.class,
|
||||
ex -> {
|
||||
exchange.getResponse().setStatusCode(ex.getStatus());
|
||||
if (ex.getMessage() != null) {
|
||||
logger.error(ex.getMessage());
|
||||
}
|
||||
return Mono.empty();
|
||||
});
|
||||
});
|
||||
|
|
|
@ -22,14 +22,11 @@ import java.util.List;
|
|||
import java.util.Map;
|
||||
import java.util.stream.Collectors;
|
||||
|
||||
import org.springframework.core.*;
|
||||
import org.springframework.web.server.ResponseStatusException;
|
||||
import reactor.core.publisher.Flux;
|
||||
import reactor.core.publisher.Mono;
|
||||
|
||||
import org.springframework.core.Conventions;
|
||||
import org.springframework.core.MethodParameter;
|
||||
import org.springframework.core.ReactiveAdapter;
|
||||
import org.springframework.core.ReactiveAdapterRegistry;
|
||||
import org.springframework.core.ResolvableType;
|
||||
import org.springframework.core.annotation.AnnotationUtils;
|
||||
import org.springframework.http.MediaType;
|
||||
import org.springframework.http.codec.HttpMessageReader;
|
||||
|
@ -154,7 +151,7 @@ public abstract class AbstractMessageReaderArgumentResolver extends HandlerMetho
|
|||
}
|
||||
|
||||
private ServerWebInputException getReadError(MethodParameter parameter, Throwable ex) {
|
||||
return new ServerWebInputException("Failed to read HTTP message", parameter, ex);
|
||||
return new ServerWebInputException("Failed to read HTTP message", parameter, ex instanceof ResponseStatusException ? ex.getCause() : ex);
|
||||
}
|
||||
|
||||
private ServerWebInputException getRequiredBodyError(MethodParameter parameter) {
|
||||
|
|
|
@ -102,7 +102,7 @@ public class DispatcherHandlerErrorTests {
|
|||
Mono<Void> publisher = this.dispatcherHandler.handle(exchange);
|
||||
|
||||
StepVerifier.create(publisher)
|
||||
.consumeErrorWith(error -> assertSame(EXCEPTION, error))
|
||||
.consumeErrorWith(error -> assertSame(EXCEPTION, error.getCause()))
|
||||
.verify();
|
||||
}
|
||||
|
||||
|
|
Loading…
Reference in New Issue