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

TypeMismatchException for @RequestParam does not contain parameter name [SPR-10153] #14786

Closed
spring-issuemaster opened this issue Jan 9, 2013 · 12 comments

Comments

Projects
None yet
2 participants
@spring-issuemaster
Copy link
Collaborator

commented Jan 9, 2013

Michael Osipov opened SPR-10153 and commented

I have defined a mapping like this:

public ResponseEntity<Object> lookupProject(@PathVariable String project,
			@RequestParam(value = "outputType", required = false, defaultValue = "HASH") OutputType outputType)

OutputType is an enum with values HASH and ARRAY.

When the converter is not able to converter to an enum a TypeMismatchException is issued. Unfortunately, TypeMismatchException#getPropertyName is not populated. I cannot reasonbly determine that exactly this parameter has failed.
Please add the appropriate PropertyChangeEvent.


Affects: 3.1.3

Issue Links:

  • #16425 MVC: which parameter cannot be parsed? ("is duplicated by")

Referenced from: commits 6f5359e

3 votes, 8 watchers

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jan 10, 2013

Michael Osipov commented

Phil, please backport this trivial issue to 3.1.x too.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jan 11, 2013

Phil Webb commented

Hi Michael,

Having looked into this a little more it seems that adding a PropertyChangeEvent to the exception is not possible. The TypeMismatchException is being thrown due to a binding error and not due to a bean property changing.

What information are you trying to extract from the error? If you need access to the @RequestMapping one option might be to obtain this via the nested ConversionFailedException. Something like this:

@ExceptionHandler
@ResponseBody
public String handleError(TypeMismatchException e) {
	return "Failed on " + ((ConversionFailedException) e.getCause()).getTargetType().getAnnotation(RequestParam.class).value();
}
@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jan 11, 2013

Michael Osipov commented

Hi Phil, thanks for the feedback. My usecase looks like this:
As you can see, I have the enum OutputType set by a request param. Now, if a client supplies a value which does not exist in the enum a TypeMismatchException is thrown. I am handling this exception with @ExceptionHandler and response with a ResponseEntity<RestError>>. The RestError object shall contain in a message string the illegal value and the parameter name of course.
I do have access to the incorrect value but not to the property, in this case a request param, which caused the error. So I do not have any qualified information to tell the client what the reason for the BAD_REQUEST is.
See ```java
@ExceptionHandler(TypeMismatchException.class)
public ResponseEntity<RestError> handleHtypeMismatchException(TypeMismatchException e) {
// FIXME Parametername nicht verfügbar aufgrund von https://jira.springsource.org/browse/SPR-10153
RestError error = new RestError(Reason.INVALID_PARAMETER, String.format(
"Der übergebene Parameterwert '%s' ist fehlerhaft/ungültig", e.getValue()), ExceptionUtils.getMessage(e));
return new ResponseEntity<RestError>(error, HttpStatus.BAD_REQUEST);
}


Reading your reply, I never knew that there is a `ConversionFailedException`. Where is the difference between both? Is  TME the right one if there is no property change? As far as I can see, this exception was made for beans and not for the web stuff. It was simply reused, unfortunately.
 I am looking for a concise way to provide parameter name and value to the client. Neither TME nor CFE provide this. If you check the `MissingServletRequestParameterException` for example, you'll see that this exception provides the exact information to present a reasonable message back to the client.
See ```java 
@ExceptionHandler(MissingServletRequestParameterException.class)
  public ResponseEntity<RestError> handleMissingServletRequestParameterException(MissingServletRequestParameterException e) {
    RestError error = new RestError(Reason.MISSING_PARAMETER, String.format(
      "Der Parameter '%s' vom Typ '%s' wurde nicht angegeben", e.getParameterName(), e.getParameterType()), ExceptionUtils.getMessage(e));
    return new ResponseEntity<RestError>(error, HttpStatus.BAD_REQUEST);
}
@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jan 14, 2013

Rossen Stoyanchev commented

What Phil is pointing out is that TypeMismatchException is the exception to catch while ConversionFailedException is the root cause. So you can do (ConversionFailedException) e.getCause(), which in turn provides the source and target of the type conversion. From the target TypeDescriptor you can extract the @RequestParam annotation. Not the most straight-forward way but it does provide what you need.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jan 14, 2013

Michael Osipov commented

Rossen,

while Phil's approach works it seems not very trivial to retrieve that name. Compared to the MissingServletRequestParameterException's getParameterName() this approach seems not very intuitive. We can turn this to a RFE to make a developer's life easier.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jan 21, 2013

Michael Osipov commented

4.0 Backlock, seriously?

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jan 21, 2013

Phil Webb commented

Hi Michael,

Unfortunately I will not have time to address this before 3.2.1 is released. I realize that the work-around is not that pretty but it should let you continue. Have you considered contributing a pull-request? It can really help expedite these kinds of issues.

https://github.com/SpringSource/spring-framework/blob/master/CONTRIBUTING.md

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jan 22, 2013

Michael Osipov commented

Hi Phil,

there is no need to rush but I have expected it atleast in 3.2 or 3.3 backlog. I would like to contribute one but how can I alter the given exception without breaking other people's code? I haven't dug that deep into the framework but that shouldn't be much of a problem with a good amount of time. If you could guide me, I would like to try.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jan 22, 2013

Phil Webb commented

The 'Fix Version/s' field is just a suggested target. We can (and often do) backport bugs like this after they have been fixed and when we can analyze any potential problems that back-porting might cause.

As you point out above fixing this issue without breaking existing code is indeed the worry. One possible solution might be to introduce a new subclass of TypeMismatchException where we could introduce a new getParameterName() method. Although TypeMismatchException.getPropertyName() initially seems like a logical place to return this information if you dig into this class you see that that getter is backed by a PropertyChangeEvent and I actually think that getParameterName() would be more consistent with MissingServletRequestParameterException.

If back compatibility was not an issue (i.e. a fix for Spring 4 only) my preference would be to introduce a completely new exception type. Something like RequestParameterTypeMismatchException that could extend ServletRequestBindingException.

Probably the place to experiment with any potential solution is in AbstractNamedValueMethodArgumentResolver.resolveArgument(). I think that the call to convertIfNecessary could catch and wrap any exceptions, including data from namedValueInfo.name.

Here is the complete stack trace for the existing error:

org.springframework.beans.TypeMismatchException: Failed to convert value of type 'java.lang.String' to required type 'org.springframework.samples.mvc.simple.OutputType'; nested exception is org.springframework.core.convert.ConversionFailedException: Failed to convert from type java.lang.String to type @org.springframework.web.bind.annotation.RequestParam org.springframework.samples.mvc.simple.OutputType for value 'ARRAYB'; nested exception is java.lang.IllegalArgumentException: No enum constant org.springframework.samples.mvc.simple.OutputType.ARRAYB
	at org.springframework.beans.TypeConverterSupport.doConvert(TypeConverterSupport.java:68)
	at org.springframework.beans.TypeConverterSupport.convertIfNecessary(TypeConverterSupport.java:45)
	at org.springframework.validation.DataBinder.convertIfNecessary(DataBinder.java:595)
	at org.springframework.web.method.annotation.AbstractNamedValueMethodArgumentResolver.resolveArgument(AbstractNamedValueMethodArgumentResolver.java:98)
	at org.springframework.web.method.support.HandlerMethodArgumentResolverComposite.resolveArgument(HandlerMethodArgumentResolverComposite.java:77)
	at org.springframework.web.method.support.InvocableHandlerMethod.getMethodArgumentValues(InvocableHandlerMethod.java:162)
	at org.springframework.web.method.support.InvocableHandlerMethod.invokeForRequest(InvocableHandlerMethod.java:123)
	at org.springframework.web.servlet.mvc.method.annotation.ServletInvocableHandlerMethod.invokeAndHandle(ServletInvocableHandlerMethod.java:104)
	at org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerAdapter.invokeHandleMethod(RequestMappingHandlerAdapter.java:745)
	at org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerAdapter.handleInternal(RequestMappingHandlerAdapter.java:686)
	at org.springframework.web.servlet.mvc.method.AbstractHandlerMethodAdapter.handle(AbstractHandlerMethodAdapter.java:80)
	at org.springframework.web.servlet.DispatcherServlet.doDispatch(DispatcherServlet.java:925)
	at org.springframework.web.servlet.DispatcherServlet.doService(DispatcherServlet.java:856)
	at org.springframework.web.servlet.FrameworkServlet.processRequest(FrameworkServlet.java:915)
	at org.springframework.web.servlet.FrameworkServlet.doGet(FrameworkServlet.java:811)
	at javax.servlet.http.HttpServlet.service(HttpServlet.java:621)
	at org.springframework.web.servlet.FrameworkServlet.service(FrameworkServlet.java:796)
	at javax.servlet.http.HttpServlet.service(HttpServlet.java:728)
	at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:305)
	at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:210)
	at org.apache.catalina.core.StandardWrapperValve.invoke(StandardWrapperValve.java:222)
	at org.apache.catalina.core.StandardContextValve.invoke(StandardContextValve.java:123)
	at org.apache.catalina.authenticator.AuthenticatorBase.invoke(AuthenticatorBase.java:472)
	at org.apache.catalina.core.StandardHostValve.invoke(StandardHostValve.java:171)
	at org.apache.catalina.valves.ErrorReportValve.invoke(ErrorReportValve.java:99)
	at org.apache.catalina.valves.AccessLogValve.invoke(AccessLogValve.java:936)
	at org.apache.catalina.core.StandardEngineValve.invoke(StandardEngineValve.java:118)
	at org.apache.catalina.connector.CoyoteAdapter.service(CoyoteAdapter.java:407)
	at org.apache.coyote.http11.AbstractHttp11Processor.process(AbstractHttp11Processor.java:1004)
	at org.apache.coyote.AbstractProtocol$AbstractConnectionHandler.process(AbstractProtocol.java:589)
	at org.apache.tomcat.util.net.JIoEndpoint$SocketProcessor.run(JIoEndpoint.java:310)
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1110)
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:603)
	at java.lang.Thread.run(Thread.java:722)
Caused by: org.springframework.core.convert.ConversionFailedException: Failed to convert from type java.lang.String to type @org.springframework.web.bind.annotation.RequestParam org.springframework.samples.mvc.simple.OutputType for value 'ARRAYB'; nested exception is java.lang.IllegalArgumentException: No enum constant org.springframework.samples.mvc.simple.OutputType.ARRAYB
	at org.springframework.core.convert.support.ConversionUtils.invokeConverter(ConversionUtils.java:41)
	at org.springframework.core.convert.support.GenericConversionService.convert(GenericConversionService.java:169)
	at org.springframework.beans.TypeConverterDelegate.convertIfNecessary(TypeConverterDelegate.java:161)
	at org.springframework.beans.TypeConverterDelegate.convertIfNecessary(TypeConverterDelegate.java:93)
	at org.springframework.beans.TypeConverterSupport.doConvert(TypeConverterSupport.java:61)
	... 33 more
Caused by: java.lang.IllegalArgumentException: No enum constant org.springframework.samples.mvc.simple.OutputType.ARRAYB
	at java.lang.Enum.valueOf(Enum.java:236)
	at org.springframework.core.convert.support.StringToEnumConverterFactory$StringToEnum.convert(StringToEnumConverterFactory.java:48)
	at org.springframework.core.convert.support.StringToEnumConverterFactory$StringToEnum.convert(StringToEnumConverterFactory.java:1)
	at org.springframework.core.convert.support.GenericConversionService$ConverterFactoryAdapter.convert(GenericConversionService.java:379)
	at org.springframework.core.convert.support.ConversionUtils.invokeConverter(ConversionUtils.java:35)
	... 37 more

Cheers,
Phil.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jan 31, 2013

Michael Osipov commented

Sorry for the late response. I highly favor a new ServletRequestParameterTypeMismatchException which would work as same as MissingServletRequestParameterException. This is unfortunate a 4.0 only. As far as I can see the TypeMismatchException was developed for beans only and has been misused here.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Mar 26, 2015

Juergen Hoeller commented

{Rossen Stoyanchev, let's revisit this one for 4.2 as well...

Juergen

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Apr 6, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.