pierrejeambrun commented on code in PR #66888:
URL: https://github.com/apache/airflow/pull/66888#discussion_r3335501810
##########
airflow-core/src/airflow/api_fastapi/common/exceptions.py:
##########
@@ -108,6 +108,38 @@ def _is_dialect_matched(self, exc: IntegrityError) -> bool:
return False
+class DataErrorHandler(BaseErrorHandler[DataError]):
+ """
+ Translate ``sqlalchemy.exc.DataError`` into a 422 response.
+
+ ``DataError`` wraps the database rejecting an INSERT/UPDATE because a value
+ exceeds the column's declared type (MySQL ``1406 Data too long``, Postgres
+ ``value too long for type``) or is out of range (MySQL ``1264 Out of range
+ value``, Postgres ``numeric field overflow``). The wrapped value came from
+ request input that passed Pydantic validation, so the failure is always a
+ client problem — translate to a 422 with the underlying database error
+ surfaced via ``orig_error`` rather than a generic 500.
+ """
+
+ def __init__(self):
+ super().__init__(DataError)
+
+ def exception_handler(self, request: Request, exc: DataError):
+ raise HTTPException(
+ status_code=status.HTTP_422_UNPROCESSABLE_ENTITY,
+ detail={
+ "reason": "Value rejected by database",
Review Comment:
"DataError, value rejected by the database". Similarly to
`_UniqueConstraintErrorHandler` we are missing some logs.
So people can debug easily.
##########
airflow-core/src/airflow/api_fastapi/common/exceptions.py:
##########
@@ -108,6 +108,38 @@ def _is_dialect_matched(self, exc: IntegrityError) -> bool:
return False
+class DataErrorHandler(BaseErrorHandler[DataError]):
+ """
+ Translate ``sqlalchemy.exc.DataError`` into a 422 response.
+
+ ``DataError`` wraps the database rejecting an INSERT/UPDATE because a value
+ exceeds the column's declared type (MySQL ``1406 Data too long``, Postgres
+ ``value too long for type``) or is out of range (MySQL ``1264 Out of range
+ value``, Postgres ``numeric field overflow``). The wrapped value came from
+ request input that passed Pydantic validation, so the failure is always a
+ client problem — translate to a 422 with the underlying database error
+ surfaced via ``orig_error`` rather than a generic 500.
+ """
+
+ def __init__(self):
+ super().__init__(DataError)
+
+ def exception_handler(self, request: Request, exc: DataError):
+ raise HTTPException(
+ status_code=status.HTTP_422_UNPROCESSABLE_ENTITY,
+ detail={
+ "reason": "Value rejected by database",
+ "statement": str(exc.statement),
+ "orig_error": str(exc.orig),
+ "message": (
+ "Database rejected the payload. Reduce the field size, or "
Review Comment:
Message should be more generic and mention the exeption_id similarly to
```
message = (
"Serious error when handling your request. Check logs
for more details - "
f"you will find it in api server when you look for ID
{exception_id}"
)
```
So it's easier to debug by reading api server logs.
##########
airflow-core/src/airflow/api_fastapi/common/exceptions.py:
##########
@@ -108,6 +108,38 @@ def _is_dialect_matched(self, exc: IntegrityError) -> bool:
return False
+class DataErrorHandler(BaseErrorHandler[DataError]):
+ """
+ Translate ``sqlalchemy.exc.DataError`` into a 422 response.
+
+ ``DataError`` wraps the database rejecting an INSERT/UPDATE because a value
+ exceeds the column's declared type (MySQL ``1406 Data too long``, Postgres
+ ``value too long for type``) or is out of range (MySQL ``1264 Out of range
+ value``, Postgres ``numeric field overflow``). The wrapped value came from
+ request input that passed Pydantic validation, so the failure is always a
+ client problem — translate to a 422 with the underlying database error
+ surfaced via ``orig_error`` rather than a generic 500.
+ """
+
+ def __init__(self):
+ super().__init__(DataError)
+
+ def exception_handler(self, request: Request, exc: DataError):
+ raise HTTPException(
+ status_code=status.HTTP_422_UNPROCESSABLE_ENTITY,
+ detail={
+ "reason": "Value rejected by database",
+ "statement": str(exc.statement),
+ "orig_error": str(exc.orig),
+ "message": (
+ "Database rejected the payload. Reduce the field size, or "
+ "ask an operator to widen the column type on MySQL "
+ "(e.g. MEDIUMTEXT / LONGTEXT)."
+ ),
+ },
+ )
Review Comment:
This is mentioning `text size` a lot, as if this was the only case of error
caught by `DataError` I think we should reword this to be more generic. THat's
just an example of a possible `DataError`
##########
airflow-core/src/airflow/api_fastapi/common/exceptions.py:
##########
@@ -108,6 +108,38 @@ def _is_dialect_matched(self, exc: IntegrityError) -> bool:
return False
+class DataErrorHandler(BaseErrorHandler[DataError]):
+ """
+ Translate ``sqlalchemy.exc.DataError`` into a 422 response.
+
+ ``DataError`` wraps the database rejecting an INSERT/UPDATE because a value
+ exceeds the column's declared type (MySQL ``1406 Data too long``, Postgres
+ ``value too long for type``) or is out of range (MySQL ``1264 Out of range
+ value``, Postgres ``numeric field overflow``). The wrapped value came from
+ request input that passed Pydantic validation, so the failure is always a
+ client problem — translate to a 422 with the underlying database error
+ surfaced via ``orig_error`` rather than a generic 500.
+ """
+
+ def __init__(self):
+ super().__init__(DataError)
+
+ def exception_handler(self, request: Request, exc: DataError):
+ raise HTTPException(
+ status_code=status.HTTP_422_UNPROCESSABLE_ENTITY,
+ detail={
+ "reason": "Value rejected by database",
+ "statement": str(exc.statement),
+ "orig_error": str(exc.orig),
Review Comment:
Statement and orig_error should be gated behind `expose_stacktrace`.
Similarly to `_UniqueConstraintErrorHandler`.
Once the gate is implemented, mirror tests with both
`@conf_vars({("api","expose_stacktrace"): "False"})` and
`@conf_vars({("api","expose_stacktrace"): "True"})`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
