Skip to content

Resolve package cycle around MissingServletRequestPartException #28455

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

Closed
jhoeller opened this issue May 12, 2022 · 1 comment
Closed

Resolve package cycle around MissingServletRequestPartException #28455

jhoeller opened this issue May 12, 2022 · 1 comment
Assignees
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: regression A bug that is also a regression
Milestone

Comments

@jhoeller
Copy link
Contributor

Since #27910 / #27948 web.multipart.MissingServletRequestPartException extends web.bind.ServletRequestBindingException which unfortunately creates a package cycle since the web.bind package depends on web.multipart within its binder implementations.

The root of the problem there is that MissingServletRequestPartException serves two rather different purposes: It is being thrown by RequestPartServletServerHttpRequest as a low-level API exception (which is why it lives in the web.multipart package), but then also used by RequestParamMethodArgumentResolver for handler method argument binding (for which it should actually live in web.bind). The proper solution would be to use a different exception type for the bind purpose, and to only let that one extend ServletRequestBindingException as of 6.0. Let's revisit this for M5.

@jhoeller jhoeller added in: web Issues in web modules (web, webmvc, webflux, websocket) type: regression A bug that is also a regression labels May 12, 2022
@jhoeller jhoeller added this to the 6.0.0-M5 milestone May 12, 2022
@rstoyanchev
Copy link
Contributor

rstoyanchev commented May 12, 2022

It looks like originally MissingServletRequestPartException came as a result of differentiating 4xx vs 5xx exception related to multipart handling, see #13284 and related fix 56c8c69, and prior to that RequestParamMethodArgumentResolver used to raise the base class ServletRequestBindingException.

I think it would be still useful to have a single exception for a missing part, no matter where it was detected. RequestPartServletServerHttpRequest is under web.multipart because it relates to multipart processing and that's the lowest place where it is needed.

One alternative would be to revert the changes under #27948, i.e. make it extend ServletException again, and document more explicitly the reason it does not extend ServletRequestBindingException is that it can be raised at a lower level and lives in a different package. In retrospect, I suspect #27910 was more a question than an actual problem. This essentially answers the "if there is no good reason" question under #27910.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: regression A bug that is also a regression
Projects
None yet
Development

No branches or pull requests

2 participants