From 011328c7930044d00f6095fbd954f25c23831154 Mon Sep 17 00:00:00 2001 From: sriram veeraghanta Date: Mon, 1 Jun 2026 15:03:22 +0530 Subject: [PATCH] [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). --- apps/api/plane/api/views/base.py | 2 +- apps/api/plane/app/views/base.py | 4 +- apps/api/plane/license/api/views/base.py | 2 +- apps/api/plane/space/views/base.py | 4 +- apps/api/plane/tests/unit/views/__init__.py | 3 + .../tests/unit/views/test_base_dispatch.py | 69 +++++++++++++++++++ 6 files changed, 78 insertions(+), 6 deletions(-) create mode 100644 apps/api/plane/tests/unit/views/__init__.py create mode 100644 apps/api/plane/tests/unit/views/test_base_dispatch.py diff --git a/apps/api/plane/api/views/base.py b/apps/api/plane/api/views/base.py index 2fb20c8d178..11e0b5a621c 100644 --- a/apps/api/plane/api/views/base.py +++ b/apps/api/plane/api/views/base.py @@ -110,7 +110,7 @@ def dispatch(self, request, *args, **kwargs): 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 diff --git a/apps/api/plane/app/views/base.py b/apps/api/plane/app/views/base.py index db5469de585..798845b22b3 100644 --- a/apps/api/plane/app/views/base.py +++ b/apps/api/plane/app/views/base.py @@ -120,7 +120,7 @@ def dispatch(self, request, *args, **kwargs): return response except Exception as exc: response = self.handle_exception(exc) - return exc + return response @property def workspace_slug(self): @@ -215,7 +215,7 @@ def dispatch(self, request, *args, **kwargs): except Exception as exc: response = self.handle_exception(exc) - return exc + return response @property def workspace_slug(self): diff --git a/apps/api/plane/license/api/views/base.py b/apps/api/plane/license/api/views/base.py index 8d0d39ac387..ea404dd4f3b 100644 --- a/apps/api/plane/license/api/views/base.py +++ b/apps/api/plane/license/api/views/base.py @@ -106,7 +106,7 @@ def dispatch(self, request, *args, **kwargs): except Exception as exc: response = self.handle_exception(exc) - return exc + return response @property def fields(self): diff --git a/apps/api/plane/space/views/base.py b/apps/api/plane/space/views/base.py index cf8cdbdc5c9..fea0a873d99 100644 --- a/apps/api/plane/space/views/base.py +++ b/apps/api/plane/space/views/base.py @@ -114,7 +114,7 @@ def dispatch(self, request, *args, **kwargs): return response except Exception as exc: response = self.handle_exception(exc) - return exc + return response @property def workspace_slug(self): @@ -197,7 +197,7 @@ def dispatch(self, request, *args, **kwargs): except Exception as exc: response = self.handle_exception(exc) - return exc + return response @property def workspace_slug(self): diff --git a/apps/api/plane/tests/unit/views/__init__.py b/apps/api/plane/tests/unit/views/__init__.py new file mode 100644 index 00000000000..fcc34a703d7 --- /dev/null +++ b/apps/api/plane/tests/unit/views/__init__.py @@ -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. diff --git a/apps/api/plane/tests/unit/views/test_base_dispatch.py b/apps/api/plane/tests/unit/views/test_base_dispatch.py new file mode 100644 index 00000000000..22e390dce51 --- /dev/null +++ b/apps/api/plane/tests/unit/views/test_base_dispatch.py @@ -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