mirror of
https://github.com/makeplane/plane.git
synced 2026-06-13 19:19:54 +00:00
[GIT-213] fix: return HTTP response from dispatch() exception handler (#9179)
* fix(api): return HTTP response from dispatch() exception handler BaseAPIView.dispatch() and BaseViewSet.dispatch() built the proper error Response via handle_exception() but returned the raw exception object instead, causing Django to raise "TypeError: 'Exception' object is not a valid HTTP response". Fix all six occurrences across the api, app, license and space view bases, and add a regression test covering every affected base class. Fixes #9157 * chore(api): add copyright header to tests/unit/views/__init__.py The empty package init file was missing the AGPL copyright header, failing the Copy Right Check CI (addlicense -check on all tracked .py files).
This commit is contained in:
committed by
GitHub
parent
3f57fefdb4
commit
011328c793
@@ -110,7 +110,7 @@ class BaseAPIView(TimezoneMixin, GenericAPIView, ReadReplicaControlMixin, BasePa
|
||||
return response
|
||||
except Exception as exc:
|
||||
response = self.handle_exception(exc)
|
||||
return exc
|
||||
return response
|
||||
|
||||
def finalize_response(self, request, response, *args, **kwargs):
|
||||
# Call super to get the default response
|
||||
|
||||
@@ -120,7 +120,7 @@ class BaseViewSet(TimezoneMixin, ReadReplicaControlMixin, ModelViewSet, BasePagi
|
||||
return response
|
||||
except Exception as exc:
|
||||
response = self.handle_exception(exc)
|
||||
return exc
|
||||
return response
|
||||
|
||||
@property
|
||||
def workspace_slug(self):
|
||||
@@ -215,7 +215,7 @@ class BaseAPIView(TimezoneMixin, ReadReplicaControlMixin, APIView, BasePaginator
|
||||
|
||||
except Exception as exc:
|
||||
response = self.handle_exception(exc)
|
||||
return exc
|
||||
return response
|
||||
|
||||
@property
|
||||
def workspace_slug(self):
|
||||
|
||||
@@ -106,7 +106,7 @@ class BaseAPIView(TimezoneMixin, APIView, BasePaginator):
|
||||
|
||||
except Exception as exc:
|
||||
response = self.handle_exception(exc)
|
||||
return exc
|
||||
return response
|
||||
|
||||
@property
|
||||
def fields(self):
|
||||
|
||||
@@ -114,7 +114,7 @@ class BaseViewSet(TimezoneMixin, ModelViewSet, BasePaginator):
|
||||
return response
|
||||
except Exception as exc:
|
||||
response = self.handle_exception(exc)
|
||||
return exc
|
||||
return response
|
||||
|
||||
@property
|
||||
def workspace_slug(self):
|
||||
@@ -197,7 +197,7 @@ class BaseAPIView(TimezoneMixin, APIView, BasePaginator):
|
||||
|
||||
except Exception as exc:
|
||||
response = self.handle_exception(exc)
|
||||
return exc
|
||||
return response
|
||||
|
||||
@property
|
||||
def workspace_slug(self):
|
||||
|
||||
@@ -0,0 +1,3 @@
|
||||
# Copyright (c) 2023-present Plane Software, Inc. and contributors
|
||||
# SPDX-License-Identifier: AGPL-3.0-only
|
||||
# See the LICENSE file for details.
|
||||
@@ -0,0 +1,69 @@
|
||||
# Copyright (c) 2023-present Plane Software, Inc. and contributors
|
||||
# SPDX-License-Identifier: AGPL-3.0-only
|
||||
# See the LICENSE file for details.
|
||||
|
||||
"""
|
||||
Regression tests for the ``dispatch()`` exception handling on the shared
|
||||
``BaseAPIView`` / ``BaseViewSet`` classes.
|
||||
|
||||
When ``super().dispatch()`` raises an unhandled exception, ``dispatch()`` must
|
||||
return the HTTP ``Response`` produced by ``handle_exception()`` -- not the raw
|
||||
exception object. Returning the exception causes Django's response pipeline to
|
||||
fail with ``TypeError: 'Exception' object is not a valid HTTP response``.
|
||||
|
||||
See: https://github.com/makeplane/plane/issues/9157
|
||||
"""
|
||||
|
||||
import pytest
|
||||
from unittest.mock import patch
|
||||
|
||||
from rest_framework import status
|
||||
from rest_framework.response import Response
|
||||
from rest_framework.test import APIRequestFactory
|
||||
|
||||
from plane.api.views.base import BaseAPIView as ApiBaseAPIView, BaseViewSet as ApiBaseViewSet
|
||||
from plane.app.views.base import BaseAPIView as AppBaseAPIView, BaseViewSet as AppBaseViewSet
|
||||
from plane.license.api.views.base import BaseAPIView as LicenseBaseAPIView
|
||||
from plane.space.views.base import BaseAPIView as SpaceBaseAPIView, BaseViewSet as SpaceBaseViewSet
|
||||
|
||||
|
||||
# Every shared base view that wraps ``super().dispatch()`` in a try/except.
|
||||
VIEW_CLASSES = [
|
||||
ApiBaseAPIView,
|
||||
ApiBaseViewSet,
|
||||
AppBaseAPIView,
|
||||
AppBaseViewSet,
|
||||
LicenseBaseAPIView,
|
||||
SpaceBaseAPIView,
|
||||
SpaceBaseViewSet,
|
||||
]
|
||||
|
||||
|
||||
@pytest.mark.unit
|
||||
@pytest.mark.parametrize(
|
||||
"view_class",
|
||||
VIEW_CLASSES,
|
||||
ids=lambda c: f"{c.__module__}.{c.__name__}",
|
||||
)
|
||||
def test_dispatch_returns_response_when_super_dispatch_raises(view_class):
|
||||
"""dispatch() must return handle_exception()'s Response, not the exception."""
|
||||
request = APIRequestFactory().get("/api/test/")
|
||||
view = view_class()
|
||||
|
||||
sentinel = Response(
|
||||
{"error": "Something went wrong please try again later"},
|
||||
status=status.HTTP_500_INTERNAL_SERVER_ERROR,
|
||||
)
|
||||
|
||||
with (
|
||||
patch("rest_framework.views.APIView.dispatch", side_effect=RuntimeError("boom")),
|
||||
patch.object(view_class, "handle_exception", return_value=sentinel) as mock_handle,
|
||||
):
|
||||
result = view.dispatch(request)
|
||||
|
||||
mock_handle.assert_called_once()
|
||||
assert isinstance(result, Response), (
|
||||
f"{view_class.__module__}.{view_class.__name__}.dispatch() returned "
|
||||
f"{type(result).__name__} instead of an HTTP Response"
|
||||
)
|
||||
assert result is sentinel
|
||||
Reference in New Issue
Block a user