Skip to content

Commit 84d90b7

Browse files
committed
Throw client exception when failing to decode field
Prior to this commit, calling `ClientResponseField.toEntity` on a valid field could throw a `DecodingException` if the requested type for deserialization is not compatible with the serialized value. When thrown in a server application, this exception is handled as an HTTP "Bad Request". This hides the server error. This commit catches all exceptions thrown during the decoding process and wraps with in `GraphQlClientException` to avoid such behavior. Closes gh-1219
1 parent 3fc0a37 commit 84d90b7

File tree

3 files changed

+56
-8
lines changed

3 files changed

+56
-8
lines changed

spring-graphql-docs/modules/ROOT/pages/client.adoc

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -249,6 +249,8 @@ include::{include-java}/client/requests/retrieve/Retrieve.java[tag=fieldError,in
249249
----
250250
======
251251

252+
If the field is present but cannot be decoded to the requested type, a plain `GraphQlClientException`
253+
is thrown instead.
252254

253255

254256
[[client.requests.execute]]

spring-graphql/src/main/java/org/springframework/graphql/client/DefaultClientResponseField.java

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -109,11 +109,15 @@ public <D> List<D> toEntityList(ParameterizedTypeReference<D> elementType) {
109109
DataBufferFactory bufferFactory = DefaultDataBufferFactory.sharedInstance;
110110
MimeType mimeType = MimeTypeUtils.APPLICATION_JSON;
111111
Map<String, Object> hints = Collections.emptyMap();
112+
try {
113+
DataBuffer buffer = ((Encoder<T>) this.response.getEncoder()).encodeValue(
114+
(T) getValue(), bufferFactory, ResolvableType.forInstance(getValue()), mimeType, hints);
112115

113-
DataBuffer buffer = ((Encoder<T>) this.response.getEncoder()).encodeValue(
114-
(T) getValue(), bufferFactory, ResolvableType.forInstance(getValue()), mimeType, hints);
115-
116-
return ((Decoder<T>) this.response.getDecoder()).decode(buffer, targetType, mimeType, hints);
116+
return ((Decoder<T>) this.response.getDecoder()).decode(buffer, targetType, mimeType, hints);
117+
}
118+
catch (Throwable ex) {
119+
throw new GraphQlClientException("Cannot read field '" + this.field.getPath() + "'", ex, this.response.getRequest());
120+
}
117121
}
118122

119123
}
Lines changed: 46 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,29 +22,30 @@
2222
import java.util.List;
2323
import java.util.Map;
2424

25+
import com.fasterxml.jackson.databind.DeserializationFeature;
26+
import com.fasterxml.jackson.databind.ObjectMapper;
2527
import graphql.GraphQLError;
2628
import graphql.GraphqlErrorBuilder;
2729
import graphql.execution.ResultPath;
2830
import graphql.language.SourceLocation;
2931
import org.jspecify.annotations.Nullable;
3032
import org.junit.jupiter.api.Test;
31-
import org.testcontainers.shaded.com.fasterxml.jackson.databind.DeserializationFeature;
32-
import org.testcontainers.shaded.com.fasterxml.jackson.databind.ObjectMapper;
3333

34+
import org.springframework.core.codec.DecodingException;
3435
import org.springframework.graphql.ResponseError;
3536
import org.springframework.http.codec.json.JacksonJsonDecoder;
3637
import org.springframework.http.codec.json.JacksonJsonEncoder;
3738

38-
3939
import static org.assertj.core.api.Assertions.assertThat;
4040
import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
41+
import static org.assertj.core.api.Assertions.assertThatThrownBy;
4142

4243

4344
/**
4445
* Unit tests for {@link DefaultClientGraphQlResponse}.
4546
* @author Rossen Stoyanchev
4647
*/
47-
class DefaultGraphQlClientResponseTests {
48+
class DefaultClientGraphQlResponseTests {
4849

4950
private static final ObjectMapper mapper = new ObjectMapper();
5051

@@ -120,6 +121,37 @@ private void testFieldValueInvalidPath(String path, String json) {
120121
.withMessageStartingWith("Invalid path");
121122
}
122123

124+
125+
@Test
126+
void fieldToEntity() throws Exception {
127+
ClientResponseField bookById = getFieldOnDataResponse("bookById", """
128+
{
129+
"bookById": {
130+
"id": "42",
131+
"name": "Spring for GraphQL"
132+
}
133+
}
134+
""");
135+
Book book = bookById.toEntity(Book.class);
136+
assertThat(book).isEqualTo(new Book(42L, "Spring for GraphQL"));
137+
}
138+
139+
@Test
140+
void fieldToEntityWhenNullThrowsFieldAccessException() throws Exception {
141+
ClientGraphQlResponse response = createResponse("{ \"bookById\": null }", createError("/bookById", "fail-book"));
142+
assertThatThrownBy(() -> response.field("bookById").toEntity(Book.class))
143+
.isInstanceOf(FieldAccessException.class).hasMessageContaining("Invalid field 'bookById'");
144+
}
145+
146+
@Test
147+
void fieldToEntityWhenInvalidThrowsGraphQlClientException() throws Exception {
148+
ClientGraphQlResponse response = createResponse("{ \"bookById\": \"invalid\" }");
149+
assertThatThrownBy(() -> response.field("bookById").toEntity(Book.class))
150+
.isInstanceOf(GraphQlClientException.class)
151+
.hasMessageContaining("Cannot read field 'bookById'")
152+
.hasCauseInstanceOf(DecodingException.class);
153+
}
154+
123155
@Test
124156
void fieldErrors() {
125157

@@ -181,11 +213,21 @@ private ClientResponseField getFieldOnErrorResponse(String path, GraphQLError...
181213
return response.field(path);
182214
}
183215

216+
private ClientGraphQlResponse createResponse(String dataJson, GraphQLError... errors) throws Exception {
217+
Map<?, ?> dataMap = mapper.readValue(dataJson, Map.class);
218+
List<?> errorList = Arrays.stream(errors).map(GraphQLError::toSpecification).toList();
219+
return createResponse(Map.of("data", dataMap, "errors", errorList));
220+
}
221+
184222
private ClientGraphQlResponse createResponse(Map<String, Object> responseMap) {
185223
return new DefaultClientGraphQlResponse(
186224
new DefaultClientGraphQlRequest("{test}", null, Collections.emptyMap(), Collections.emptyMap(), Collections.emptyMap()),
187225
new ResponseMapGraphQlResponse(responseMap),
188226
new JacksonJsonEncoder(), new JacksonJsonDecoder());
189227
}
190228

229+
record Book(Long id, String name) {
230+
231+
}
232+
191233
}

0 commit comments

Comments
 (0)