-
Notifications
You must be signed in to change notification settings - Fork 38.5k
MappingJackson2HttpMessageConverter should not always log a warning [SPR-14163] #18735
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
Comments
Juergen Hoeller commented I suppose we could add a flag to Have you tried ordering your converters accordingly? So that if the Protobuf converter comes first, the Jackson converter wouldn't even see those types? |
Claes Mogren commented I tried that first, the complete code block for the restTemplate is: @Bean
public RestTemplate restTemplate(ProtobufHttpMessageConverter protobufHttpMessageConverter) {
final RestTemplate restTemplate = new RestTemplate();
restTemplate.getMessageConverters().add(protobufHttpMessageConverter);
final MappingJackson2HttpMessageConverter jsonMessageConverter = new MappingJackson2HttpMessageConverter();
final ObjectMapper objectMapper = new ObjectMapper();
objectMapper.configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false);
jsonMessageConverter.setObjectMapper(objectMapper);
restTemplate.getMessageConverters().add(jsonMessageConverter);
restTemplate.setRequestFactory(new HttpComponentsClientHttpRequestFactory());
return restTemplate;
}
@Bean
public ProtobufHttpMessageConverter protobufHttpMessageConverter() {
return new ProtobufHttpMessageConverter();
} I still get the error. I did open a PR to just not always show the error: I just think having if (logger.isDebugEnabled()) {
logger.warn()
} else {
logger.warn()
} in the canRead() and canWrite() methods is strange. |
Juergen Hoeller commented We're using such an As for the need to log at warn level to begin with, the problem here is that Jackson itself may fail to introspect a Jackson-targeted class. If this remains unnoticed, message conversion simply doesn't happen using Jackson and there is no indication why, just a "no message converter found" exception eventually. Debug level is too low in such a scenario. Have you tried replacing the list through |
Claes Mogren commented Ok, thanks a lot for the help with this. I tried
Which is also correct, the error is that we take a json, and serialize it directly into a protobuf class. Then I tried creating a list and using
I guess I might have to create a complete testcase for this. What app does is that it has a REST endpoint that returns a protobuf with some configuration, that comes from another service as JSON. So what the app does is basically return restTemplate.exchange(url, HttpMethod.GET, entity, clazz).getBody(); where clazz is the protobuf generated class. It works fine, just annoying to fill the logs. Is the problem that we use the |
Claes Mogren commented I closed the PR since it was not the correct way to solve it. But, as far as I could see, the other |
Juergen Hoeller commented Ideally, we'd be able to differentiate between a Jackson exception indicating "this looks like a Jackson-mapped class but has invalid metadata" versus "this doesn't look like a Jackson-intended mapping at all", adapting the log level accordingly. What does the stacktrace of your |
Juergen Hoeller commented Actually, reviewing the code, there's one thing we can do in any case: Check the media type first before delegating to Jackson for evaluating the given type, i.e. call |
Claes Mogren commented Debug log:
And yes, backing out immediately sounds like it would help. |
Juergen Hoeller commented Alright, the immediate backing out if the media type doesn't match is already available in the latest The |
Claes Mogren commented Thanks, I'm afraid the flag is the way to go. Running with the debug-flag correctly set this time, I still get the Warning in the logs: 2016-04-14 09:57:55.073 WARN [app-service,] 20214 --- [appClient-4] .c.j.MappingJackson2HttpMessageConverter : Failed to evaluate deserialization for type [simple type, class com.company.product.middleware.app.App$Configurations] com.fasterxml.jackson.databind.JsonMappingException: Can not find a (Map) Key deserializer for type [simple type, class com.google.protobuf.Descriptors$FieldDescriptor] Debugging I can see the following values for the parameters: type: class com.company.product.middleware.app.App$Configuration
contextClass: null
mediaType: null
javaType: "[simple type class com.company.product.middleware.app.App$Configuration]" I could also see that |
Claes Mogren commented By reading the code in |
Juergen Hoeller commented In normal operations, Jackson doesn't expose an exception there, so we're not actually logging anything anyway. It's just that for nested value scenarios, Jackson seems to go into a code path where it forgets that it was originally just asked whether it can handle a particular type and suddently throws hard exceptions: Compare So to be clear, the only reason why we do warn logging at all there is to handle scenarios where Jackson would otherwise just return |
Juergen Hoeller commented I've added explicit introspection of the |
Claes Mogren commented Thanks a lot for the fix! We'll move over to 4.3 as soon as it's ready. |
Juergen Hoeller commented It'd be great if you could give the latest Beyond snapshots, there will be a 4.3 RC2 release on April 28th, as a more definitive state to test against. |
Claes Mogren commented I rebuilt the application with |
Claes Mogren opened SPR-14163 and commented
We have a project that registers both a MappingJackson2HttpMessageConverter and a ProtobufHttpMessageConverter to our RestTemplate. This works fine, but every time we de-serialize a JSON body into a protobuf we get something like:
2016-04-13 13:22:30.760 WARN [app-service] 15947 --- [AppClient-1] .c.j.MappingJackson2HttpMessageConverter : Failed to evaluate deserialization for type [simple type, class com.company.product.component.protobuf.Client$Configurations]: com.fasterxml.jackson.databind.JsonMappingException: Can not find a (Map) Key deserializer for type [simple type, class com.google.protobuf.Descriptors$FieldDescriptor]
It can of course be hidden by setting logging.level.org.springframework.http.converter.json: 'ERROR' in our application.yml, but it's not the solution.
Affects: 4.1.7
Reference URL: https://github.com/spring-projects/spring-framework/blob/master/spring-web/src/main/java/org/springframework/http/converter/json/AbstractJackson2HttpMessageConverter.java#L156
Issue Links:
Referenced from: commits 62ce9af, bf3cadb, e366746, 5f4e838, 334b4a9
The text was updated successfully, but these errors were encountered: