Refine encoding/decoding exception handling

Starting with removing a package cycle on the use of
ResponseStatusException in the codec package, this commit generally
refines codec exception handling.

The new [Encoding|Decoding]Exception mirror the existing
HttpMessageNot[Readable|Writable]Exception and are used similarly
especially to differentiate betwen 400 and 500 errors when parsing
server request body content.

The commit also aligns some of the exception handling of JSON and XML
on the WebFlux side with that on the Spring MVC side.

Issue: SPR-15516

spring-core/src/main/java/org/springframework/core/codec/CodecException.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2016 the original author or authors.
+ * Copyright 2002-2017 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -19,18 +19,29 @@ package org.springframework.core.codec;
 import org.springframework.core.NestedRuntimeException;
 
 /**
- * Codec related exception, usually used as a wrapper for a cause exception.
+ * General error that indicates a problem while encoding and decoding to and
+ * from an Object stream.
  *
  * @author Sebastien Deleuze
+ * @author Rossen Stoyanchev
  * @since 5.0
  */
 @SuppressWarnings("serial")
 public class CodecException extends NestedRuntimeException {
 
+	/**
+	 * Create a new CodecException.
+	 * @param msg the detail message
+	 */
 	public CodecException(String msg) {
 		super(msg);
 	}
 
+	/**
+	 * Create a new CodecException.
+	 * @param msg the detail message
+	 * @param cause root cause for the exception, if any
+	 */
 	public CodecException(String msg, Throwable cause) {
 		super(msg, cause);
 	}

spring-core/src/main/java/org/springframework/core/codec/DecodingException.java

@@ -0,0 +1,52 @@
+/*
+ * Copyright 2002-2017 the original author or authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.springframework.core.codec;
+
+/**
+ * Indicates an issue with decoding the input stream with the focus on indicating
+ * a content issue such as a parse failure. As opposed to a more general I/O
+ * errors, illegal state, or a {@link CodecException} such as a configuration
+ * issue that a {@link Decoder} may choose to raise.
+ *
+ * <p>For example in a web application, a server side {@code DecodingException}
+ * would translate to a response with a 400 (bad input) status while
+ * {@code CodecException} would translate to 500 (server error) status.
+ *
+ * @author Rossen Stoyanchev
+ * @since 5.0
+ * @see Decoder
+ */
+@SuppressWarnings("serial")
+public class DecodingException extends CodecException {
+
+	/**
+	 * Create a new DecodingException.
+	 * @param msg the detail message
+	 */
+	public DecodingException(String msg) {
+		super(msg);
+	}
+
+	/**
+	 * Create a new DecodingException.
+	 * @param msg the detail message
+	 * @param cause root cause for the exception, if any
+	 */
+	public DecodingException(String msg, Throwable cause) {
+		super(msg, cause);
+	}
+
+}

spring-core/src/main/java/org/springframework/core/codec/EncodingException.java

@@ -13,24 +13,35 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-
 package org.springframework.core.codec;
 
 /**
- * Codec exception suitable for internal errors, like those not related to invalid data. It can be used to make sure
- * such error will produce a 5xx status code and not a 4xx one when reading HTTP messages for example.
+ * Indicates an issue with encoding the input Object stream with a focus on
+ * indicating the Objects cannot be encoded. As opposed to a more general
+ * {@link CodecException} such as a configuration issue that an {@link Encoder}
+ * may also choose to raise.
  *
- * @author Sebastien Deleuze
+ * @author Rossen Stoyanchev
  * @since 5.0
+ * @see Encoder
  */
 @SuppressWarnings("serial")
-public class InternalCodecException extends CodecException {
+public class EncodingException extends CodecException {
 
-	public InternalCodecException(String msg) {
+	/**
+	 * Create a new EncodingException.
+	 * @param msg the detail message
+	 */
+	public EncodingException(String msg) {
 		super(msg);
 	}
 
-	public InternalCodecException(String msg, Throwable cause) {
+	/**
+	 * Create a new EncodingException.
+	 * @param msg the detail message
+	 * @param cause root cause for the exception, if any
+	 */
+	public EncodingException(String msg, Throwable cause) {
 		super(msg, cause);
 	}
 

spring-web/src/main/java/org/springframework/http/codec/DecoderHttpMessageReader.java

@@ -21,9 +21,6 @@ import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 
-import org.springframework.core.codec.InternalCodecException;
-import org.springframework.http.HttpStatus;
-import org.springframework.web.server.ResponseStatusException;
 import reactor.core.publisher.Flux;
 import reactor.core.publisher.Mono;
 
@@ -88,9 +85,7 @@ public class DecoderHttpMessageReader<T> implements HttpMessageReader<T> {
 			Map<String, Object> hints) {
 
 		MediaType contentType = getContentType(message);
-		return this.decoder
-				.decode(message.getBody(), elementType, contentType, hints)
-				.onErrorMap(this::mapError);
+		return this.decoder.decode(message.getBody(), elementType, contentType, hints);
 	}
 
 	@Override
@@ -98,9 +93,7 @@ public class DecoderHttpMessageReader<T> implements HttpMessageReader<T> {
 			Map<String, Object> hints) {
 
 		MediaType contentType = getContentType(message);
-		return this.decoder
-				.decodeToMono(message.getBody(), elementType, contentType, hints)
-				.onErrorMap(this::mapError);
+		return this.decoder.decodeToMono(message.getBody(), elementType, contentType, hints);
 	}
 
 	private MediaType getContentType(HttpMessage inputMessage) {
@@ -108,16 +101,6 @@ 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 InternalCodecException) {
-			return new ResponseStatusException(HttpStatus.INTERNAL_SERVER_ERROR, "Failed to decode HTTP message", ex);
-		}
-		return new ResponseStatusException(HttpStatus.BAD_REQUEST, "Failed to decode HTTP message", ex);
-	}
-
 
 	// Server-side only...
 

spring-web/src/main/java/org/springframework/http/codec/EncoderHttpMessageWriter.java

@@ -22,14 +22,13 @@ 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;
 
 import org.springframework.core.ResolvableType;
 import org.springframework.core.codec.Encoder;
 import org.springframework.core.io.buffer.DataBuffer;
+import org.springframework.core.io.buffer.DataBufferFactory;
 import org.springframework.http.MediaType;
 import org.springframework.http.ReactiveHttpOutputMessage;
 import org.springframework.http.server.reactive.ServerHttpRequest;
@@ -97,10 +96,9 @@ public class EncoderHttpMessageWriter<T> implements HttpMessageWriter<T> {
 			Map<String, Object> hints) {
 
 		MediaType contentType = updateContentType(message, mediaType);
+		DataBufferFactory factory = message.bufferFactory();
 
-		Flux<DataBuffer> body = this.encoder
-				.encode(inputStream, message.bufferFactory(), elementType, contentType, hints)
-				.onErrorMap(this::mapError);
+		Flux<DataBuffer> body = this.encoder.encode(inputStream, factory, elementType, contentType, hints);
 
 		return isStreamingMediaType(contentType) ?
 				message.writeAndFlushWith(body.map(Flux::just)) :
@@ -139,13 +137,6 @@ 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...
 

spring-web/src/main/java/org/springframework/http/codec/json/Jackson2JsonDecoder.java

@@ -21,6 +21,7 @@ import java.lang.annotation.Annotation;
 import java.util.List;
 import java.util.Map;
 
+import com.fasterxml.jackson.core.JsonProcessingException;
 import com.fasterxml.jackson.databind.JavaType;
 import com.fasterxml.jackson.databind.ObjectMapper;
 import com.fasterxml.jackson.databind.ObjectReader;
@@ -32,10 +33,10 @@ import reactor.core.publisher.Mono;
 import org.springframework.core.MethodParameter;
 import org.springframework.core.ResolvableType;
 import org.springframework.core.codec.CodecException;
+import org.springframework.core.codec.DecodingException;
 import org.springframework.core.io.buffer.DataBuffer;
 import org.springframework.core.io.buffer.DataBufferUtils;
 import org.springframework.http.codec.HttpMessageDecoder;
-import org.springframework.core.codec.InternalCodecException;
 import org.springframework.http.converter.json.Jackson2ObjectMapperBuilder;
 import org.springframework.http.server.reactive.ServerHttpRequest;
 import org.springframework.http.server.reactive.ServerHttpResponse;
@@ -116,10 +117,13 @@ public class Jackson2JsonDecoder extends Jackson2CodecSupport implements HttpMes
 						return value;
 					}
 					catch (InvalidDefinitionException ex) {
-						throw new InternalCodecException("Error while reading the data", ex);
+						throw new CodecException("Type definition error: " + ex.getMessage(), ex);
+					}
+					catch (JsonProcessingException ex) {
+						throw new DecodingException("JSON parse error: " + ex.getMessage(), ex);
 					}
 					catch (IOException ex) {
-						throw new CodecException("Error while reading the data", ex);
+						throw new CodecException("I/O error while reading: " + ex.getMessage(), ex);
 					}
 				});
 	}

spring-web/src/main/java/org/springframework/http/codec/json/Jackson2JsonEncoder.java

@@ -24,6 +24,7 @@ import java.util.Collections;
 import java.util.List;
 import java.util.Map;
 
+import com.fasterxml.jackson.core.JsonProcessingException;
 import com.fasterxml.jackson.core.PrettyPrinter;
 import com.fasterxml.jackson.core.util.DefaultIndenter;
 import com.fasterxml.jackson.core.util.DefaultPrettyPrinter;
@@ -39,6 +40,7 @@ import reactor.core.publisher.Mono;
 import org.springframework.core.MethodParameter;
 import org.springframework.core.ResolvableType;
 import org.springframework.core.codec.CodecException;
+import org.springframework.core.codec.EncodingException;
 import org.springframework.core.io.buffer.DataBuffer;
 import org.springframework.core.io.buffer.DataBufferFactory;
 import org.springframework.http.MediaType;
@@ -164,8 +166,11 @@ public class Jackson2JsonEncoder extends Jackson2CodecSupport implements HttpMes
 		try {
 			writer.writeValue(outputStream, value);
 		}
+		catch (JsonProcessingException ex) {
+			throw new EncodingException("JSON encoding error: " + ex.getMessage(), ex);
+		}
 		catch (IOException ex) {
-			throw new CodecException("Error while writing the data", ex);
+			throw new CodecException("I/O error while writing: " + ex.getMessage(), ex);
 		}
 
 		return buffer;

spring-web/src/main/java/org/springframework/http/codec/xml/Jaxb2XmlDecoder.java

@@ -37,7 +37,7 @@ import reactor.core.publisher.Mono;
 
 import org.springframework.core.ResolvableType;
 import org.springframework.core.codec.AbstractDecoder;
-import org.springframework.core.codec.CodecException;
+import org.springframework.core.codec.DecodingException;
 import org.springframework.core.io.buffer.DataBuffer;
 import org.springframework.util.ClassUtils;
 import org.springframework.util.MimeType;
@@ -216,7 +216,7 @@ public class Jaxb2XmlDecoder extends AbstractDecoder<Object> {
 			}
 		}
 		catch (JAXBException ex) {
-			throw new CodecException(ex.getMessage(), ex);
+			throw new DecodingException(ex.getMessage(), ex);
 		}
 	}
 

spring-web/src/main/java/org/springframework/http/codec/xml/Jaxb2XmlEncoder.java

@@ -21,6 +21,7 @@ import java.nio.charset.StandardCharsets;
 import java.util.Map;
 import javax.xml.bind.JAXBException;
 import javax.xml.bind.Marshaller;
+import javax.xml.bind.UnmarshalException;
 import javax.xml.bind.annotation.XmlRootElement;
 import javax.xml.bind.annotation.XmlType;
 
@@ -28,6 +29,8 @@ import reactor.core.publisher.Flux;
 
 import org.springframework.core.ResolvableType;
 import org.springframework.core.codec.AbstractSingleValueEncoder;
+import org.springframework.core.codec.CodecException;
+import org.springframework.core.codec.EncodingException;
 import org.springframework.core.io.buffer.DataBuffer;
 import org.springframework.core.io.buffer.DataBufferFactory;
 import org.springframework.util.ClassUtils;
@@ -73,13 +76,16 @@ public class Jaxb2XmlEncoder extends AbstractSingleValueEncoder<Object> {
 			OutputStream outputStream = buffer.asOutputStream();
 			Class<?> clazz = ClassUtils.getUserClass(value);
 			Marshaller marshaller = jaxbContexts.createMarshaller(clazz);
-			marshaller
-					.setProperty(Marshaller.JAXB_ENCODING, StandardCharsets.UTF_8.name());
+			marshaller.setProperty(Marshaller.JAXB_ENCODING, StandardCharsets.UTF_8.name());
 			marshaller.marshal(value, outputStream);
 			return Flux.just(buffer);
 		}
+		catch (UnmarshalException ex) {
+			return Flux.error(new EncodingException(
+					"Could not unmarshal to [" + value.getClass() + "]: " + ex.getMessage(), ex));
+		}
 		catch (JAXBException ex) {
-			return Flux.error(ex);
+			return Flux.error(new CodecException("Could not instantiate JAXBContext: " + ex.getMessage(), ex));
 		}
 	}
 

spring-web/src/test/java/org/springframework/http/codec/json/Jackson2JsonDecoderTests.java

@@ -19,30 +19,31 @@ package org.springframework.http.codec.json;
 import java.util.List;
 import java.util.Map;
 
-import static java.util.Arrays.asList;
-import static java.util.Collections.*;
-
 import com.fasterxml.jackson.databind.ObjectMapper;
 import org.junit.Test;
-
-import static org.springframework.core.ResolvableType.forClass;
-import static org.springframework.http.MediaType.*;
-import static org.springframework.http.codec.json.Jackson2JsonDecoder.*;
-import static org.springframework.http.codec.json.JacksonViewBean.*;
 import reactor.core.publisher.Flux;
 import reactor.core.publisher.Mono;
 import reactor.test.StepVerifier;
 
 import org.springframework.core.ResolvableType;
 import org.springframework.core.codec.CodecException;
-import org.springframework.core.codec.InternalCodecException;
+import org.springframework.core.codec.DecodingException;
 import org.springframework.core.io.buffer.AbstractDataBufferAllocatingTestCase;
 import org.springframework.core.io.buffer.DataBuffer;
 import org.springframework.http.codec.Pojo;
 
+import static java.util.Arrays.asList;
+import static java.util.Collections.emptyMap;
+import static java.util.Collections.singletonMap;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
+import static org.springframework.core.ResolvableType.forClass;
+import static org.springframework.http.MediaType.APPLICATION_JSON;
+import static org.springframework.http.MediaType.APPLICATION_XML;
+import static org.springframework.http.codec.json.Jackson2JsonDecoder.JSON_VIEW_HINT;
+import static org.springframework.http.codec.json.JacksonViewBean.MyJacksonView1;
+import static org.springframework.http.codec.json.JacksonViewBean.MyJacksonView3;
 
 /**
  * Unit tests for {@link Jackson2JsonDecoder}.
@@ -168,7 +169,7 @@ public class Jackson2JsonDecoderTests extends AbstractDataBufferAllocatingTestCa
 		Flux<DataBuffer> source = Flux.just(stringBuffer( "{\"foofoo\": \"foofoo\", \"barbar\": \"barbar\"}"));
 		ResolvableType elementType = forClass(Pojo.class);
 		Flux<Object> flux = new Jackson2JsonDecoder(new ObjectMapper()).decode(source, elementType, null, emptyMap());
-		StepVerifier.create(flux).verifyErrorMatches(ex -> ex instanceof CodecException && !(ex instanceof InternalCodecException));
+		StepVerifier.create(flux).verifyErrorMatches(ex -> ex instanceof DecodingException);
 	}
 
 	@Test
@@ -176,9 +177,7 @@ public class Jackson2JsonDecoderTests extends AbstractDataBufferAllocatingTestCa
 		Flux<DataBuffer> source = Flux.just(stringBuffer( "{\"property1\":\"foo\",\"property2\":\"bar\"}"));
 		ResolvableType elementType = forClass(BeanWithNoDefaultConstructor.class);
 		Flux<Object> flux = new Jackson2JsonDecoder().decode(source, elementType, null, emptyMap());
-		StepVerifier
-				.create(flux)
-				.expectError(InternalCodecException.class);
+		StepVerifier.create(flux).verifyError(CodecException.class);
 	}
 
 

spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/AbstractMessageReaderArgumentResolver.java

@@ -17,19 +17,22 @@
 package org.springframework.web.reactive.result.method.annotation;
 
 import java.lang.annotation.Annotation;
+import java.lang.reflect.Method;
 import java.util.Collections;
 import java.util.List;
 import java.util.Map;
 import java.util.stream.Collectors;
 
-import org.springframework.core.*;
-import org.springframework.core.codec.InternalCodecException;
-import org.springframework.http.HttpStatus;
-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.core.codec.DecodingException;
 import org.springframework.http.MediaType;
 import org.springframework.http.codec.HttpMessageReader;
 import org.springframework.http.server.reactive.ServerHttpRequest;
@@ -117,9 +120,9 @@ public abstract class AbstractMessageReaderArgumentResolver extends HandlerMetho
 				Map<String, Object> readHints = Collections.emptyMap();
 				if (adapter != null && adapter.isMultiValue()) {
 					Flux<?> flux = reader.read(bodyType, elementType, request, response, readHints);
-					flux = flux.onErrorResume(ex -> Flux.error(getReadError(bodyParameter, ex)));
+					flux = flux.onErrorResume(ex -> Flux.error(handleReadError(bodyParameter, ex)));
 					if (isBodyRequired || !adapter.supportsEmpty()) {
-						flux = flux.switchIfEmpty(Flux.error(getRequiredBodyError(bodyParameter)));
+						flux = flux.switchIfEmpty(Flux.error(handleMissingBody(bodyParameter)));
 					}
 					Object[] hints = extractValidationHints(bodyParameter);
 					if (hints != null) {
@@ -130,9 +133,9 @@ public abstract class AbstractMessageReaderArgumentResolver extends HandlerMetho
 				}
 				else {
 					Mono<?> mono = reader.readMono(bodyType, elementType, request, response, readHints);
-					mono = mono.onErrorResume(ex -> Mono.error(getReadError(bodyParameter, ex)));
+					mono = mono.onErrorResume(ex -> Mono.error(handleReadError(bodyParameter, ex)));
 					if (isBodyRequired || (adapter != null && !adapter.supportsEmpty())) {
-						mono = mono.switchIfEmpty(Mono.error(getRequiredBodyError(bodyParameter)));
+						mono = mono.switchIfEmpty(Mono.error(handleMissingBody(bodyParameter)));
 					}
 					Object[] hints = extractValidationHints(bodyParameter);
 					if (hints != null) {
@@ -152,17 +155,14 @@ public abstract class AbstractMessageReaderArgumentResolver extends HandlerMetho
 		return Mono.error(new UnsupportedMediaTypeStatusException(mediaType, this.supportedMediaTypes));
 	}
 
-	private ResponseStatusException getReadError(MethodParameter parameter, Throwable ex) {
-		Throwable cause = ex instanceof ResponseStatusException ? ex.getCause() : ex;
-
-		return cause instanceof InternalCodecException ?
-				new ResponseStatusException(HttpStatus.INTERNAL_SERVER_ERROR, "Failed to read HTTP message", cause) :
-				new ServerWebInputException("Failed to read HTTP message", parameter, cause);
+	private Throwable handleReadError(MethodParameter parameter, Throwable ex) {
+		return ex instanceof DecodingException ?
+				new ServerWebInputException("Failed to read HTTP message", parameter, ex) : ex;
 	}
 
-	private ServerWebInputException getRequiredBodyError(MethodParameter parameter) {
-		return new ServerWebInputException("Required request body is missing: " +
-				parameter.getMethod().toGenericString());
+	private ServerWebInputException handleMissingBody(MethodParameter parameter) {
+		Method method = parameter.getMethod();
+		return new ServerWebInputException("Request body is missing: " + method.toGenericString());
 	}
 
 	/**

spring-webflux/src/test/java/org/springframework/web/reactive/DispatcherHandlerErrorTests.java

@@ -44,7 +44,6 @@ import org.springframework.web.reactive.result.method.annotation.ResponseBodyRes
 import org.springframework.web.server.NotAcceptableStatusException;
 import org.springframework.web.server.ResponseStatusException;
 import org.springframework.web.server.ServerWebExchange;
-import org.springframework.web.server.ServerWebInputException;
 import org.springframework.web.server.WebExceptionHandler;
 import org.springframework.web.server.WebHandler;
 import org.springframework.web.server.handler.ExceptionHandlingWebHandler;
@@ -52,8 +51,10 @@ import org.springframework.web.server.handler.ExceptionHandlingWebHandler;
 import static org.hamcrest.CoreMatchers.instanceOf;
 import static org.hamcrest.CoreMatchers.startsWith;
 import static org.hamcrest.Matchers.is;
-import static org.junit.Assert.*;
-import static org.springframework.http.MediaType.*;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertSame;
+import static org.junit.Assert.assertThat;
+import static org.springframework.http.MediaType.APPLICATION_JSON;
 
 /**
  * Test the effect of exceptions at different stages of request processing by
@@ -98,7 +99,7 @@ public class DispatcherHandlerErrorTests {
 		Mono<Void> publisher = this.dispatcherHandler.handle(exchange);
 
 		StepVerifier.create(publisher)
-				.consumeErrorWith(error -> assertSame(EXCEPTION, error.getCause()))
+				.consumeErrorWith(error -> assertSame(EXCEPTION, error))
 				.verify();
 	}
 
@@ -147,10 +148,7 @@ public class DispatcherHandlerErrorTests {
 		Mono<Void> publisher = this.dispatcherHandler.handle(exchange);
 
 		StepVerifier.create(publisher)
-				.consumeErrorWith(error -> {
-					assertThat(error, instanceOf(ServerWebInputException.class));
-					assertSame(EXCEPTION, error.getCause());
-				})
+				.consumeErrorWith(error -> assertSame(EXCEPTION, error))
 				.verify();
 	}