-
Notifications
You must be signed in to change notification settings - Fork 6k
Java spring throw exceptions (#5686) #5738
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
Java spring throw exceptions (#5686) #5738
Conversation
@stevecookform3 your PR contains 2 commits not belonging to you.
I would suggest you to rebase the change on the latest 2.3.0 so as to resolve the merge conflicts (or file a new one based on the latest 2.3.0)
eaf0886 to
ebf1ede
Compare
@wing328 i've rebased to 2.3.0. can you take another look? thanks..
@wing328 sorry let me fix the commit and test first
@stevecookform3 no worry. Please take your time and let us know if you need any help.
Most importantly, thanks for the enhancement 👍
7c71f51 to
fdc5e41
Compare
fdc5e41 to
15a0bf5
Compare
@wing328 fixed. can you take a look?
@stevecookform3 I've resolved the merge conflicts and merged your PR into 2.3.0 Please pull the latest to have a look. Thanks.
Oh, too late it seems.
I think throwing Exception in interfaces is a bad pattern : it defeats the checked exception pattern and is just like using Object in your methods.
Checked exceptions should be handled in the implementations and if you want external handling (eg. ControllerAdvice/ExceptionHandler), you can rethrow runtime exceptions.
@cbornet well never too later to voice out :). Do you mind opening a new issue to start the discussion?
Other server generators (e.g. Nancyfx) may need similar debate later so maybe we need a CLI option to cater both?
Global exception handling is generally not the right approach, and by default the generation should definitely NOT throw Exception. If anything, this should be a configurable option for those few that still use global exception handling.
Not all breaking changes are good changes... :-)
@Bols Understood that not everyone likes global exception handling. Do you mind contributing the configurable option for this?
@Bols yup, please pull the latest master to give it a try.
PR checklist
./bin/to update Petstore sample so that CIs can verify the change. (For instance, only need to run./bin/{LANG}-petstore.shand./bin/security/{LANG}-petstore.shif updating the {LANG} (e.g. php, ruby, python, etc) code generator or {LANG} client's mustache templates)2.3.0branch for breaking (non-backward compatible) changes.Description of the PR
Adds 'throws Exception' to all controller methods. This allows servers to implement global exception handling in Spring, where controller methods can throw exceptions and have them handled globally.
Breaking change, hence filed against 2.3.0
See Issue #5686