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

Update CrnkErrorController to be compatible with Spring Boot >= 2.3.X #821

Merged

Conversation

datagitlies
Copy link
Contributor

The getErrorAttributes method that the CrnkErrorController was calling had been deprecated in Spring Boot 2.3 and then was removed in Spring Boot 2.5 - this pull request updates the controller to use the new / non-deprecated version of the getErrorAttributes method.

Closes #816

@remmeier
Copy link
Contributor

if server.error.include-message=always.is always reqired,.maybe it should be added to the spring configuration to enable it by default?

@datagitlies
Copy link
Contributor Author

if server.error.include-message=always.is always reqired,.maybe it should be added to the spring configuration to enable it by default?

I only set the value to fix a broken test. I think crnk should be agnostic to this setting. The main thing is formatting the error per the JSON:API specification. The end user can then decide to include the message, or not, depending on their needs using the spring property. Crnk need not take a stance either way which I would think is best.

@datagitlies
Copy link
Contributor Author

@remmeier is there anything you'd like me to change? Also, I'm curious if/when this and a few other fixes could be rolled up into a new release... I'm sort of stuck waiting on this pull request as well as #815

@rohitb1985
Copy link

In which release of Crnk can we expect this to be fixed?

@Incanus3
Copy link

Incanus3 commented Sep 29, 2021

I can confirm that this PR solves the issue. Tried in a small test project using the --include-build ../crnk-framework gradle option. The only problem (except having to downgrade gradle) I have when testing this way is that I can't use the crnk-validations module, because it fails with class file for javax.validation.ValidatorFactory not found, even though when running with crnk.io as maven dependency, I don't encounter this problem, but that is most probably some kind of class resolving problem and definitely not related to this issue. If you know, how to test this better though, please give me a hint.

@remmeier
Copy link
Contributor

remmeier commented Oct 1, 2021

ehatbteat is failing? if itnis an essential one, it erver.error.include-message=always should become a default. or else the flag docunented maybe. else let usbstash and merge then.

@datagitlies
Copy link
Contributor Author

datagitlies commented Oct 4, 2021

@remmeier from my understanding, the CrnkErrorController that is "broken" here is not being used in any of the core pieces of the crnk-framework. It only applies to applications using Spring Boot / Spring MVC. In my case, this error controller simply formats the response for anything other than my resource / relationship repository URLs. Simple things like a 404 coming back from Spring's DispatcherServlet are then formatted according to the spec. Very nice to have but not critical. It looks like this issue is preventing several users of crnk from upgrading their Spring Boot version to 2.5.X (myself included). Hence, we're all just wondering if/when this fix can be made and a new version of crnk released.

@datagitlies
Copy link
Contributor Author

@remmeier is there anything you'd like me to change? Also, I'm curious if/when this and a few other fixes could be rolled up into a new release... I'm sort of stuck waiting on this pull request as well as #815

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Crnk error handling is not compatible with Spring Boot 2.5
4 participants