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.

Sign up for GitHub

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

Jump to bottom

fix: trap SQLAlchemy common exceptions & throw 422 error instead #19672

Merged
merged 1 commit into from
Apr 20, 2022
Merged

fix: trap SQLAlchemy common exceptions & throw 422 error instead #19672

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion superset/views/base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@
from flask_wtf.csrf import CSRFError
from flask_wtf.form import FlaskForm
from pkg_resources import resource_filename
from sqlalchemy import or_
from sqlalchemy import exc, or_
from sqlalchemy.orm import Query
from werkzeug.exceptions import HTTPException
from wtforms import Form
Expand Down Expand Up @@ -231,6 +231,9 @@ def wraps(self: "BaseSupersetView", *args: Any, **kwargs: Any) -> FlaskResponse:
return json_error_response(
utils.error_msg_from_exception(ex), status=cast(int, ex.code)
)
except (exc.IntegrityError, exc.DatabaseError, exc.DataError) as ex:
logger.exception(ex)
return json_error_response(utils.error_msg_from_exception(ex), status=422)
Comment on lines +234 to +236
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Returning 422 status code for DatabaseError and IntegrityError is not good. Because these errors are not related to the entity which is sent from Client side.

500 is more acceptable for these errors.
422 is good for DataError

Check here:
https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/422
https://docs.sqlalchemy.org/en/14/errors.html

except Exception as ex: # pylint: disable=broad-except
logger.exception(ex)
return json_error_response(utils.error_msg_from_exception(ex))
Expand Down
9 changes: 9 additions & 0 deletions superset/views/base_api.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
from superset.stats_logger import BaseStatsLogger
from superset.superset_typing import FlaskResponse
from superset.utils.core import time_function
from superset.views.base import handle_api_exception

logger = logging.getLogger(__name__)
get_related_schema = {
Expand Down Expand Up @@ -386,6 +387,7 @@ def send_stats_metrics(
object_ref=False,
log_to_statsd=False,
)
@handle_api_exception
def info_headless(self, **kwargs: Any) -> Response:
"""
Add statsd metrics to builtin FAB _info endpoint
Expand All @@ -399,6 +401,7 @@ def info_headless(self, **kwargs: Any) -> Response:
object_ref=False,
log_to_statsd=False,
)
@handle_api_exception
def get_headless(self, pk: int, **kwargs: Any) -> Response:
"""
Add statsd metrics to builtin FAB GET endpoint
Expand All @@ -412,6 +415,7 @@ def get_headless(self, pk: int, **kwargs: Any) -> Response:
object_ref=False,
log_to_statsd=False,
)
@handle_api_exception
def get_list_headless(self, **kwargs: Any) -> Response:
"""
Add statsd metrics to builtin FAB GET list endpoint
Expand All @@ -425,6 +429,7 @@ def get_list_headless(self, **kwargs: Any) -> Response:
object_ref=False,
log_to_statsd=False,
)
@handle_api_exception
def post_headless(self) -> Response:
"""
Add statsd metrics to builtin FAB POST endpoint
Expand All @@ -438,6 +443,7 @@ def post_headless(self) -> Response:
object_ref=False,
log_to_statsd=False,
)
@handle_api_exception
def put_headless(self, pk: int) -> Response:
"""
Add statsd metrics to builtin FAB PUT endpoint
Expand All @@ -451,6 +457,7 @@ def put_headless(self, pk: int) -> Response:
object_ref=False,
log_to_statsd=False,
)
@handle_api_exception
def delete_headless(self, pk: int) -> Response:
"""
Add statsd metrics to builtin FAB DELETE endpoint
Expand All @@ -464,6 +471,7 @@ def delete_headless(self, pk: int) -> Response:
@safe
@statsd_metrics
@rison(get_related_schema)
@handle_api_exception
def related(self, column_name: str, **kwargs: Any) -> FlaskResponse:
"""Get related fields data
---
Expand Down Expand Up @@ -542,6 +550,7 @@ def related(self, column_name: str, **kwargs: Any) -> FlaskResponse:
@safe
@statsd_metrics
@rison(get_related_schema)
@handle_api_exception
def distinct(self, column_name: str, **kwargs: Any) -> FlaskResponse:
"""Get distinct values from field data
---
Expand Down