From 42b1e7143e48b5a4042c42278eba15adf0bf2337 Mon Sep 17 00:00:00 2001 From: Arthur Goldberg Date: Mon, 5 Apr 2021 17:01:48 +0200 Subject: [PATCH] deprecate abort (#2077) --- sanic/exceptions.py | 85 ++++++++++++++++------------------- tests/test_asgi.py | 1 - tests/test_exceptions.py | 27 ++++++++--- tests/test_middleware.py | 2 +- tests/test_redirect.py | 2 +- tests/test_request_stream.py | 1 - tests/test_request_timeout.py | 13 +----- tests/test_response.py | 5 --- tests/test_timeout_logic.py | 1 - 9 files changed, 61 insertions(+), 76 deletions(-) diff --git a/sanic/exceptions.py b/sanic/exceptions.py index f21a296c..1b823e8b 100644 --- a/sanic/exceptions.py +++ b/sanic/exceptions.py @@ -3,26 +3,18 @@ from typing import Optional, Union from sanic.helpers import STATUS_CODES -_sanic_exceptions = {} - - -def add_status_code(code, quiet=None): - """ - Decorator used for adding exceptions to :class:`SanicException`. - """ - - def class_decorator(cls): - cls.status_code = code - if quiet or quiet is None and code != 500: - cls.quiet = True - _sanic_exceptions[code] = cls - return cls - - return class_decorator - - class SanicException(Exception): - def __init__(self, message, status_code=None, quiet=None): + def __init__( + self, + message: Optional[Union[str, bytes]] = None, + status_code: Optional[int] = None, + quiet: Optional[bool] = None, + ) -> None: + + if message is None and status_code is not None: + msg: bytes = STATUS_CODES.get(status_code, b"") + message = msg.decode("utf8") + super().__init__(message) if status_code is not None: @@ -33,45 +25,42 @@ class SanicException(Exception): self.quiet = True -@add_status_code(404) class NotFound(SanicException): """ **Status**: 404 Not Found """ - pass + status_code = 404 -@add_status_code(400) class InvalidUsage(SanicException): """ **Status**: 400 Bad Request """ - pass + status_code = 400 -@add_status_code(405) class MethodNotSupported(SanicException): """ **Status**: 405 Method Not Allowed """ + status_code = 405 + def __init__(self, message, method, allowed_methods): super().__init__(message) self.headers = {"Allow": ", ".join(allowed_methods)} -@add_status_code(500) class ServerError(SanicException): """ **Status**: 500 Internal Server Error """ - pass + status_code = 500 -@add_status_code(503) class ServiceUnavailable(SanicException): """ **Status**: 503 Service Unavailable @@ -80,7 +69,7 @@ class ServiceUnavailable(SanicException): down for maintenance). Generally, this is a temporary state. """ - pass + status_code = 503 class URLBuildError(ServerError): @@ -88,7 +77,7 @@ class URLBuildError(ServerError): **Status**: 500 Internal Server Error """ - pass + status_code = 500 class FileNotFound(NotFound): @@ -102,7 +91,6 @@ class FileNotFound(NotFound): self.relative_url = relative_url -@add_status_code(408) class RequestTimeout(SanicException): """The Web server (running the Web site) thinks that there has been too long an interval of time between 1) the establishment of an IP @@ -112,16 +100,15 @@ class RequestTimeout(SanicException): server has 'timed out' on that particular socket connection. """ - pass + status_code = 408 -@add_status_code(413) class PayloadTooLarge(SanicException): """ **Status**: 413 Payload Too Large """ - pass + status_code = 413 class HeaderNotFound(InvalidUsage): @@ -129,36 +116,35 @@ class HeaderNotFound(InvalidUsage): **Status**: 400 Bad Request """ - pass + status_code = 400 -@add_status_code(416) class ContentRangeError(SanicException): """ **Status**: 416 Range Not Satisfiable """ + status_code = 416 + def __init__(self, message, content_range): super().__init__(message) self.headers = {"Content-Range": f"bytes */{content_range.total}"} -@add_status_code(417) class HeaderExpectationFailed(SanicException): """ **Status**: 417 Expectation Failed """ - pass + status_code = 417 -@add_status_code(403) class Forbidden(SanicException): """ **Status**: 403 Forbidden """ - pass + status_code = 403 class InvalidRangeType(ContentRangeError): @@ -166,7 +152,7 @@ class InvalidRangeType(ContentRangeError): **Status**: 416 Range Not Satisfiable """ - pass + status_code = 416 class PyFileError(Exception): @@ -174,7 +160,6 @@ class PyFileError(Exception): super().__init__("could not execute config file %s", file) -@add_status_code(401) class Unauthorized(SanicException): """ **Status**: 401 Unauthorized @@ -210,6 +195,8 @@ class Unauthorized(SanicException): realm="Restricted Area") """ + status_code = 401 + def __init__(self, message, status_code=None, scheme=None, **kwargs): super().__init__(message, status_code) @@ -241,9 +228,13 @@ def abort(status_code: int, message: Optional[Union[str, bytes]] = None): :param status_code: The HTTP status code to return. :param message: The HTTP response body. Defaults to the messages in """ - if message is None: - msg: bytes = STATUS_CODES[status_code] - # These are stored as bytes in the STATUS_CODES dict - message = msg.decode("utf8") - sanic_exception = _sanic_exceptions.get(status_code, SanicException) - raise sanic_exception(message=message, status_code=status_code) + import warnings + + warnings.warn( + "sanic.exceptions.abort has been marked as deprecated, and will be " + "removed in release 21.12.\n To migrate your code, simply replace " + "abort(status_code, msg) with raise SanicException(msg, status_code), " + "or even better, raise an appropriate SanicException subclass." + ) + + raise SanicException(message=message, status_code=status_code) diff --git a/tests/test_asgi.py b/tests/test_asgi.py index 1859560d..a4632537 100644 --- a/tests/test_asgi.py +++ b/tests/test_asgi.py @@ -1,5 +1,4 @@ import asyncio -import sys from collections import deque, namedtuple diff --git a/tests/test_exceptions.py b/tests/test_exceptions.py index a8e00ea3..e6683ed2 100644 --- a/tests/test_exceptions.py +++ b/tests/test_exceptions.py @@ -1,15 +1,18 @@ +import warnings + import pytest from bs4 import BeautifulSoup from sanic import Sanic from sanic.exceptions import ( + SanicException, Forbidden, InvalidUsage, NotFound, ServerError, Unauthorized, - abort, + abort ) from sanic.response import text @@ -68,16 +71,19 @@ def exception_app(): @app.route("/abort/401") def handler_401_error(request): - abort(401) + raise SanicException(status_code=401) @app.route("/abort") def handler_500_error(request): + raise SanicException(status_code=500) + + @app.route("/old_abort") + def handler_old_abort_error(request): abort(500) - return text("OK") @app.route("/abort/message") def handler_abort_message(request): - abort(500, message="Abort") + raise SanicException(message="Custom Message", status_code=500) @app.route("/divide_by_zero") def handle_unhandled_exception(request): @@ -208,14 +214,21 @@ def test_exception_in_exception_handler_debug_on(exception_app): assert response.body.startswith(b"Exception raised in exception ") -def test_abort(exception_app): - """Test the abort function""" +def test_sanic_exception(exception_app): + """Test sanic exceptions are handled""" request, response = exception_app.test_client.get("/abort/401") assert response.status == 401 request, response = exception_app.test_client.get("/abort") assert response.status == 500 + # check fallback message + assert "Internal Server Error" in response.text request, response = exception_app.test_client.get("/abort/message") assert response.status == 500 - assert "Abort" in response.text + assert "Custom Message" in response.text + + with warnings.catch_warnings(record=True) as w: + request, response = exception_app.test_client.get("/old_abort") + assert response.status == 500 + assert len(w) == 1 and 'deprecated' in w[0].message.args[0] diff --git a/tests/test_middleware.py b/tests/test_middleware.py index 0d0ca5ec..a6c893b4 100644 --- a/tests/test_middleware.py +++ b/tests/test_middleware.py @@ -3,7 +3,7 @@ import logging from asyncio import CancelledError from itertools import count -from sanic.exceptions import NotFound, SanicException +from sanic.exceptions import NotFound from sanic.request import Request from sanic.response import HTTPResponse, text diff --git a/tests/test_redirect.py b/tests/test_redirect.py index 984139a1..85375cc3 100644 --- a/tests/test_redirect.py +++ b/tests/test_redirect.py @@ -1,4 +1,4 @@ -from urllib.parse import quote, unquote +from urllib.parse import quote import pytest diff --git a/tests/test_request_stream.py b/tests/test_request_stream.py index e39524c6..dd0b3530 100644 --- a/tests/test_request_stream.py +++ b/tests/test_request_stream.py @@ -8,7 +8,6 @@ import pytest from sanic import Sanic from sanic.blueprints import Blueprint from sanic.response import json, text -from sanic.server import HttpProtocol from sanic.views import CompositionView, HTTPMethodView from sanic.views import stream as stream_decorator diff --git a/tests/test_request_timeout.py b/tests/test_request_timeout.py index f60edeaa..f099d165 100644 --- a/tests/test_request_timeout.py +++ b/tests/test_request_timeout.py @@ -1,21 +1,10 @@ import asyncio -from typing import cast import httpcore import httpx -from httpcore._async.base import ( - AsyncByteStream, - AsyncHTTPTransport, - ConnectionState, - NewConnectionRequired, -) -from httpcore._async.connection import AsyncHTTPConnection -from httpcore._async.connection_pool import ResponseByteStream -from httpcore._exceptions import LocalProtocolError, UnsupportedProtocol -from httpcore._types import TimeoutDict -from httpcore._utils import url_to_origin + from sanic_testing.testing import SanicTestClient from sanic import Sanic diff --git a/tests/test_response.py b/tests/test_response.py index 366465e0..b03da996 100644 --- a/tests/test_response.py +++ b/tests/test_response.py @@ -1,23 +1,19 @@ import asyncio import inspect import os -import warnings from collections import namedtuple from mimetypes import guess_type from random import choice -from unittest.mock import MagicMock from urllib.parse import unquote import pytest from aiofiles import os as async_os -from sanic_testing.testing import HOST, PORT from sanic import Sanic from sanic.response import ( HTTPResponse, - StreamingHTTPResponse, empty, file, file_stream, @@ -26,7 +22,6 @@ from sanic.response import ( stream, text, ) -from sanic.server import HttpProtocol JSON_DATA = {"ok": True} diff --git a/tests/test_timeout_logic.py b/tests/test_timeout_logic.py index 74324cdd..05249f11 100644 --- a/tests/test_timeout_logic.py +++ b/tests/test_timeout_logic.py @@ -1,6 +1,5 @@ import asyncio -from time import monotonic as current_time from unittest.mock import Mock import pytest