quarkus-bot[bot]
commented
1 month ago /cc @DavideD (hibernate-reactive), @gavinking (hibernate-reactive)
Closed cvgaviao closed 1 month ago
quarkus-bot[bot]
commented
1 month ago /cc @DavideD (hibernate-reactive), @gavinking (hibernate-reactive)
geoand
commented
1 month ago Actually Quarkus handles jakarta.validation.ConstraintViolationException and turns it into an HTTP 400 response. It does not handleorg.hibernate.exception.ConstraintViolationException`.
I personally believe this behavior is correct, but I'll let @yrodiere comment more.
DavideD
commented
1 month ago I think 500 is a better default: the request wasn't bad, you could have run it in a different database and it would have worked. But I guess it's open to interpretation and I'm not an HTTP expert.
That said, the quick-start for Hibernate Reactive Panache shows how you can remap an error to a response using an ExceptionMapper.
DavideD
commented
1 month ago When we get an org.hibernate.exception.ConstraintViolationException exception I was expecting to get it wrapped into a 400 error,
By the way, are you expecting error 400 because it works that way somewhere else, or it used to behave that way in the past?
yrodiere
commented
1 month ago I personally believe this behavior is correct, but I'll let @yrodiere comment more.
IMO org.hibernate.exception.ConstraintViolationException has just too many possible meanings to map it to a specific HTTP status code.
It could be caused by:
I could see 1. being mapped to HTTP 403 Already Exists, or HTTP 409 Conflict, but 403 definitely would not work for 3., and it's highly questionable whether 409 would work for either 2. or 3.
Beyond that, you'd expect a REST API to provide error messages that API users can understand, relate to, and use to fix their request. I very much doubt the kind of message returned by DB violations would qualify.
Really, a better way of doing this would be to have a try/catch block in methods where you expect such a constraint violation in the normal operation of your application, and to throw an application exception (or return a custom response) with a clear message.
By the way, are you expecting error 400 because it works that way somewhere else, or it used to behave that way in the past?
+1, this is the core of the issue.
How it should behave can be debated, but a strong argument would be "it works this way in other major frameworks, and here are their reasons".
cvgaviao
commented
1 month ago By the way, are you expecting error 400 because it works that way somewhere else, or it used to behave that way in the past?
Well, if I'm not wrong almost all Constraint violations that I got with Quarkus was mapped to 400
HTTP/1.1 400 Bad Request
content-length: 135
Content-Type: application/json;charset=UTF-8
validation-exception: true
{
"title": "Constraint Violation",
"status": 400,
"violations": [
{
"field": "update.arg1.surname",
"message": "size must be between 3 and 150"
}
]
}And I think that any constraint fits better the 4xx than 5xx, since it is a client error and not a server one.
4xx client errors
404 error on Wikimedia
This class of status code is intended for situations in which the error seems to have been caused by the client. Except when responding to a HEAD request, the server should include an entity containing an explanation of the error situation, and whether it is a temporary or permanent condition. These status codes are applicable to any [request method](https://en.wikipedia.org/wiki/HTTP#Request_methods). User agents should displayReally, a better way of doing this would be to have a try/catch block in methods where you expect such a constraint violation in the normal operation of your application, and to throw an application exception (or return a custom response) with a clear message.
Well, would be. but its a kind of trick to have try/catch with Mutiny :)
Perhaps, we could have a kind of onFailure() that we could replace the original exception with another one, if necessary ?
yrodiere
commented
1 month ago Well, if I'm not wrong almost all Constraint violations that I got with Quarkus was mapped to 400
That's constraint violations in Hibernate Validator (jakarta.validation.ConstraintViolationException), a library designed precisely to validate user input, so HTTP 400 makes sense in that case.
org.hibernate.exception.ConstraintViolationException comes from Hibernate ORM (or Reactive), a library designed to provide access to the database. And in this case the exception is straight up copied from the error returned by the database.
Quite a different situation.
Well, would be. but its a kind of trick to have try/catch with Mutiny :)
I'm no Mutiny expert, but: https://smallrye.io/smallrye-mutiny/latest/tutorials/handling-failures/#transforming-failures
.onFailure().transform(originalException -> new MyException(originalException));@jponge or @cescoffier will correct me if I'm wrong.
geoand
commented
1 month ago That's constraint violations in Hibernate Validator (jakarta.validation.ConstraintViolationException), a library designed precisely to validate user input, so HTTP 400 makes sense in that case.
+1
org.hibernate.exception.ConstraintViolationException comes from Hibernate ORM (or Reactive), a library designed to provide access to the database. And in this case the exception is straight up copied from the error returned by the database.
Quite a different situation.
+1
I think we agree that what is being done currently is the proper default behavior, so I am going to close this (although that doesn't mean we can't continue the discussion :))
Describe the bug
When we try to delete an object which can't be deleted due a referential constraint (the object is being used in another table) we are supposed to receive a
org.hibernate.exception.ConstraintViolationException, and I'm receiving it indeed.The problem is that I was expecting to get an error 400, but instead I'm getting 500.
Expected behavior
When we get an
org.hibernate.exception.ConstraintViolationExceptionexception I was expecting to get it wrapped into a 400 error,Actual behavior
I'm getting a 500 error instead of 400:
How to Reproduce?
Run the quarkus:test and watch the result of
com.mycompany.api.app_mngt.UserResourceTest.testDeleteFailedByConstraint()quarkus-restlink-and-db-constraint-issue.zipOutput of
uname -aorverwindows 10
Output of
java -versionopenjdk version "21" 2023-09-19 OpenJDK Runtime Environment (build 21+35-2513) OpenJDK 64-Bit Server VM (build 21+35-2513, mixed mode, sharing)
Quarkus version or git rev
3.15.1
Build tool (ie. output of
mvnw --versionorgradlew --version)Apache Maven 3.9.9 (8e8579a9e76f7d015ee5ec7bfcdc97d260186937) Maven home: C:\Users\cristianoga\tools\apache-maven-3.9.9 Java version: 21, vendor: Oracle Corporation, runtime: C:\Users\cristianoga\tools\jdk\jdk-21 Default locale: en_US, platform encoding: UTF-8 OS name: "windows 10", version: "10.0", arch: "amd64", family: "windows"
Additional information
No response