Skip to content
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

Need Graceful Handling for Exceptions #606

Closed
raghuraman1 opened this issue Apr 24, 2020 · 11 comments
Closed

Need Graceful Handling for Exceptions #606

raghuraman1 opened this issue Apr 24, 2020 · 11 comments
Labels
enhancement New feature or request

Comments

@raghuraman1
Copy link

Hi,

Lets say I add a field of type MonetaryAmount as shown below into same Person class code I shared to you last time:
@Currency("USD")
private MonetaryAmount someField;
//not showing getters and setters here
I tried different dependencies version variations and various things.
image

I believe MonetaryAmount has always been a thorn for swagger and springdoc.
Thats ok.
This is not about MonetaryAmount. Its in general when something fails similarly.
@bnasslahsen My point is can we make the functionality fail more gracefully. At least show the controllers,models that did not fail and for what fails have a graceful message. It will have to be done in api-docs I think.

Currently When i visit http://localhost:8080/swagger-ui.html it causes below mentioned problems.
A) this screenshot
image

B) an exception in console as show below:
java.lang.IllegalArgumentException: Conflicting setter definitions for property "number": javax.money.MonetaryAmountFactory#setNumber(1 params) vs javax.money.MonetaryAmountFactory#setNumber(1 params)
at com.fasterxml.jackson.databind.introspect.POJOPropertyBuilder.getSetter(POJOPropertyBuilder.java:497) ~[jackson-databind-2.10.1.jar:2.10.1]
at com.fasterxml.jackson.databind.introspect.BeanPropertyDefinition.getMutator(BeanPropertyDefinition.java:203) ~[jackson-databind-2.10.1.jar:2.10.1]
at com.fasterxml.jackson.databind.introspect.POJOPropertyBuilder.getPrimaryMember(POJOPropertyBuilder.java:571) ~[jackson-databind-2.10.1.jar:2.10.1]
at io.swagger.v3.core.jackson.ModelResolver.resolve(ModelResolver.java:544) ~[swagger-core-2.1.2.jar:2.1.2]
at org.springdoc.core.converters.AdditionalModelsConverter.resolve(AdditionalModelsConverter.java:60) ~[classes/:na]
at org.springdoc.core.converters.PropertyCustomizingConverter.resolve(PropertyCustomizingConverter.java:42) ~[classes/:na]
at org.springdoc.core.converters.FileSupportConverter.resolve(FileSupportConverter.java:44) ~[classes/:na]
at org.springdoc.core.converters.ResponseSupportConverter.resolve(ResponseSupportConverter.java:56) ~[classes/:na]
at io.swagger.v3.core.converter.ModelConverterContextImpl.resolve(ModelConverterContextImpl.java:90) ~[swagger-core-2.1.2.jar:2.1.2]
at io.swagger.v3.core.jackson.ModelResolver.resolve(ModelResolver.java:655) ~[swagger-core-2.1.2.jar:2.1.2]
at org.springdoc.core.converters.AdditionalModelsConverter.resolve(AdditionalModelsConverter.java:60) ~[classes/:na]
at org.springdoc.core.converters.PropertyCustomizingConverter.resolve(PropertyCustomizingConverter.java:42) ~[classes/:na]
at org.springdoc.core.converters.FileSupportConverter.resolve(FileSupportConverter.java:44) ~[classes/:na]
at org.springdoc.core.converters.ResponseSupportConverter.resolve(ResponseSupportConverter.java:56) ~[classes/:na]
at io.swagger.v3.core.converter.ModelConverterContextImpl.resolve(ModelConverterContextImpl.java:90) ~[swagger-core-2.1.2.jar:2.1.2]
at io.swagger.v3.core.jackson.ModelResolver.resolve(ModelResolver.java:655) ~[swagger-core-2.1.2.jar:2.1.2]
at org.springdoc.core.converters.AdditionalModelsConverter.resolve(AdditionalModelsConverter.java:60) ~[classes/:na]
at org.springdoc.core.converters.PropertyCustomizingConverter.resolve(PropertyCustomizingConverter.java:42) ~[classes/:na]
at org.springdoc.core.converters.FileSupportConverter.resolve(FileSupportConverter.java:44) ~[classes/:na]
at org.springdoc.core.converters.ResponseSupportConverter.resolve(ResponseSupportConverter.java:56) ~[classes/:na]
at io.swagger.v3.core.converter.ModelConverterContextImpl.resolve(ModelConverterContextImpl.java:90) ~[swagger-core-2.1.2.jar:2.1.2]
at io.swagger.v3.core.converter.ModelConverters.resolveAsResolvedSchema(ModelConverters.java:112) ~[swagger-core-2.1.2.jar:2.1.2]
at org.springdoc.core.SpringDocAnnotationsUtils.extractSchema(SpringDocAnnotationsUtils.java:59) ~[classes/:na]
at org.springdoc.core.SpringDocAnnotationsUtils.resolveSchemaFromType(SpringDocAnnotationsUtils.java:47) ~[classes/:na]
at org.springdoc.core.GenericParameterBuilder.calculateSchema(GenericParameterBuilder.java:206) ~[classes/:na]
at org.springdoc.core.RequestBodyBuilder.buildRequestBody(RequestBodyBuilder.java:122) ~[classes/:na]
at org.springdoc.core.RequestBodyBuilder.calculateRequestBodyInfo(RequestBodyBuilder.java:103) ~[classes/:na]
at org.springdoc.core.AbstractRequestBuilder.build(AbstractRequestBuilder.java:199) ~[classes/:na]
at org.springdoc.api.AbstractOpenApiResource.calculatePath(AbstractOpenApiResource.java:229) ~[classes/:na]
at org.springdoc.webmvc.api.OpenApiResource.calculatePath(OpenApiResource.java:142) ~[classes/:na]
at org.springdoc.webmvc.api.OpenApiResource.getPaths(OpenApiResource.java:108) ~[classes/:na]
at org.springdoc.api.AbstractOpenApiResource.getOpenApi(AbstractOpenApiResource.java:146) ~[classes/:na]
at org.springdoc.webmvc.api.OpenApiResource.openapiJson(OpenApiResource.java:92) ~[classes/:na]
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) ~[na:1.8.0_171]
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) ~[na:1.8.0_171]
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) ~[na:1.8.0_171]
at java.lang.reflect.Method.invoke(Method.java:498) ~[na:1.8.0_171]
at org.springframework.web.method.support.InvocableHandlerMethod.doInvoke(InvocableHandlerMethod.java:190) ~[spring-web-5.2.2.RELEASE.jar:5.2.2.RELEASE]
at org.springframework.web.method.support.InvocableHandlerMethod.invokeForRequest(InvocableHandlerMethod.java:138) ~[spring-web-5.2.2.RELEASE.jar:5.2.2.RELEASE]
at org.springframework.web.servlet.mvc.method.annotation.ServletInvocableHandlerMethod.invokeAndHandle(ServletInvocableHandlerMethod.java:106) ~[spring-webmvc-5.2.2.RELEASE.jar:5.2.2.RELEASE]
at org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerAdapter.invokeHandlerMethod(RequestMappingHandlerAdapter.java:888) ~[spring-webmvc-5.2.2.RELEASE.jar:5.2.2.RELEASE]
at org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerAdapter.handleInternal(RequestMappingHandlerAdapter.java:793) ~[spring-webmvc-5.2.2.RELEASE.jar:5.2.2.RELEASE]
at org.springframework.web.servlet.mvc.method.AbstractHandlerMethodAdapter.handle(AbstractHandlerMethodAdapter.java:87) ~[spring-webmvc-5.2.2.RELEASE.jar:5.2.2.RELEASE]
at org.springframework.web.servlet.DispatcherServlet.doDispatch(DispatcherServlet.java:1040) ~[spring-webmvc-5.2.2.RELEASE.jar:5.2.2.RELEASE]
at org.springframework.web.servlet.DispatcherServlet.doService(DispatcherServlet.java:943) ~[spring-webmvc-5.2.2.RELEASE.jar:5.2.2.RELEASE]
at org.springframework.web.servlet.FrameworkServlet.processRequest(FrameworkServlet.java:1006) [spring-webmvc-5.2.2.RELEASE.jar:5.2.2.RELEASE]
at org.springframework.web.servlet.FrameworkServlet.doGet(FrameworkServlet.java:898) [spring-webmvc-5.2.2.RELEASE.jar:5.2.2.RELEASE]
at javax.servlet.http.HttpServlet.service(HttpServlet.java:634) [tomcat-embed-core-9.0.29.jar:9.0.29]
at org.springframework.web.servlet.FrameworkServlet.service(FrameworkServlet.java:883) [spring-webmvc-5.2.2.RELEASE.jar:5.2.2.RELEASE]
at javax.servlet.http.HttpServlet.service(HttpServlet.java:741) [tomcat-embed-core-9.0.29.jar:9.0.29]
at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:231) [tomcat-embed-core-9.0.29.jar:9.0.29]
at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:166) [tomcat-embed-core-9.0.29.jar:9.0.29]
at org.apache.tomcat.websocket.server.WsFilter.doFilter(WsFilter.java:53) [tomcat-embed-websocket-9.0.29.jar:9.0.29]
at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:193) [tomcat-embed-core-9.0.29.jar:9.0.29]
at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:166) [tomcat-embed-core-9.0.29.jar:9.0.29]
at org.springframework.web.filter.RequestContextFilter.doFilterInternal(RequestContextFilter.java:100) [spring-web-5.2.2.RELEASE.jar:5.2.2.RELEASE]
at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:119) [spring-web-5.2.2.RELEASE.jar:5.2.2.RELEASE]
at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:193) [tomcat-embed-core-9.0.29.jar:9.0.29]
at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:166) [tomcat-embed-core-9.0.29.jar:9.0.29]
at org.springframework.web.filter.FormContentFilter.doFilterInternal(FormContentFilter.java:93) [spring-web-5.2.2.RELEASE.jar:5.2.2.RELEASE]
at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:119) [spring-web-5.2.2.RELEASE.jar:5.2.2.RELEASE]
at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:193) [tomcat-embed-core-9.0.29.jar:9.0.29]
at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:166) [tomcat-embed-core-9.0.29.jar:9.0.29]
at org.springframework.web.filter.CharacterEncodingFilter.doFilterInternal(CharacterEncodingFilter.java:201) [spring-web-5.2.2.RELEASE.jar:5.2.2.RELEASE]
at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:119) [spring-web-5.2.2.RELEASE.jar:5.2.2.RELEASE]
at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:193) [tomcat-embed-core-9.0.29.jar:9.0.29]
at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:166) [tomcat-embed-core-9.0.29.jar:9.0.29]
at org.apache.catalina.core.StandardWrapperValve.invoke(StandardWrapperValve.java:202) [tomcat-embed-core-9.0.29.jar:9.0.29]
at org.apache.catalina.core.StandardContextValve.invoke(StandardContextValve.java:96) [tomcat-embed-core-9.0.29.jar:9.0.29]
at org.apache.catalina.authenticator.AuthenticatorBase.invoke(AuthenticatorBase.java:526) [tomcat-embed-core-9.0.29.jar:9.0.29]
at org.apache.catalina.core.StandardHostValve.invoke(StandardHostValve.java:139) [tomcat-embed-core-9.0.29.jar:9.0.29]
at org.apache.catalina.valves.ErrorReportValve.invoke(ErrorReportValve.java:92) [tomcat-embed-core-9.0.29.jar:9.0.29]
at org.apache.catalina.core.StandardEngineValve.invoke(StandardEngineValve.java:74) [tomcat-embed-core-9.0.29.jar:9.0.29]
at org.apache.catalina.connector.CoyoteAdapter.service(CoyoteAdapter.java:343) [tomcat-embed-core-9.0.29.jar:9.0.29]
at org.apache.coyote.http11.Http11Processor.service(Http11Processor.java:367) [tomcat-embed-core-9.0.29.jar:9.0.29]
at org.apache.coyote.AbstractProcessorLight.process(AbstractProcessorLight.java:65) [tomcat-embed-core-9.0.29.jar:9.0.29]
at org.apache.coyote.AbstractProtocol$ConnectionHandler.process(AbstractProtocol.java:860) [tomcat-embed-core-9.0.29.jar:9.0.29]
at org.apache.tomcat.util.net.NioEndpoint$SocketProcessor.doRun(NioEndpoint.java:1591) [tomcat-embed-core-9.0.29.jar:9.0.29]
at org.apache.tomcat.util.net.SocketProcessorBase.run(SocketProcessorBase.java:49) [tomcat-embed-core-9.0.29.jar:9.0.29]
at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149) [na:1.8.0_171]
at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624) [na:1.8.0_171]
at org.apache.tomcat.util.threads.TaskThread$WrappingRunnable.run(TaskThread.java:61) [tomcat-embed-core-9.0.29.jar:9.0.29]
at java.lang.Thread.run(Thread.java:748) [na:1.8.0_171]

@bnasslahsen
Copy link
Contributor

Hi @raghuraman1,

This enhancement will be added for the next release.
For MonetaryAmount, the easiest way is to define your own ModelConverter.
Here is a sample converter:

@Bean
public MonetaryAmountConverter samplePropertyConverter(){
	return new MonetaryAmountConverter();
}

public class MonetaryAmountConverter implements ModelConverter {

@Override
public Schema resolve(AnnotatedType type, ModelConverterContext context, Iterator<ModelConverter> chain) {
	if (type.isSchemaProperty()) {
		JavaType _type = Json.mapper().constructType(type.getType());
		if (_type != null) {
			Class<?> cls = _type.getRawClass();
		if (MonetaryAmount.class.isAssignableFrom(cls)) {
			return new ObjectSchema()
					.addProperties("amount", new NumberSchema()) // optionally add format
					.addProperties("currency", new StringSchema());
			}
		}
	}
	return (chain.hasNext()) ? chain.next().resolve(type, context, chain) : null;
}
}

@raghuraman1
Copy link
Author

Great. Thanks. @bnasslahsen

@raghuraman1
Copy link
Author

raghuraman1 commented Apr 26, 2020

@bnasslahsen
Do you have any code snippet for reverse.
I wrote a deserializer. It works nicely.
What I dont like is response is too verbose. Its showing the whole graph which is quite a lot.
Maybe Mixins can help. Will try.
image
Any suggestion on reducing the response noise.

@bnasslahsen
Copy link
Contributor

Hi @raghuraman1,

If you are having the MonetaryAmount not as field, the straight forward solution is the following (no need of MonetaryAmountConverter):

static {
    SpringDocUtils.getConfig().replaceWithClass(MonetaryAmount.class, CustomMonetaryAmount.class);
}

public class CustomMonetaryAmount {

	@JsonProperty("amount")
	private BigDecimal amount;

	@JsonProperty("currency")
	private String currency;

}

I will see if we can make it out of the box.

@raghuraman1
Copy link
Author

@bnasslahsen
Tried it using what you committed. Does serve the purpose well.
Thanks

@raghuraman1
Copy link
Author

raghuraman1 commented Apr 26, 2020

Hi @bnasslahsen
Its not perfect. There is one problem with this (Referring to the converter). Will create a new ticket and I do have a fix.
R

@raghuraman1
Copy link
Author

Hi @bnasslahsen
If I do the PR hope you will help me complete it as me this time. :)
But we must first see if you like what I propose.
R

@bnasslahsen
Copy link
Contributor

Sure, just make sure you push everything, with the tests included :)

@raghuraman1
Copy link
Author

Sure once you approve will add the tests.

@bnasslahsen
Copy link
Contributor

every change (PR) needs a test ...

@raghuraman1
Copy link
Author

OK. Will add the tests. Do look at the solution approach also.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants