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.

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

Web exceptions refactoring #3462

Merged
merged 16 commits into from
Jan 14, 2019
3 changes: 3 additions & 0 deletions .editorconfig
Original file line number Diff line number Diff line change
Expand Up @@ -17,3 +17,6 @@ indent_style = tab

[*.{yml,yaml}]
indent_size = 2

[*.rst]
max_line_length = 80
1 change: 1 addition & 0 deletions CHANGES/3462.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
``web.HTTPException`` and derived classes are not inherited from ``web.Response`` anymore.
146 changes: 110 additions & 36 deletions aiohttp/web_exceptions.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,12 @@
import warnings
from typing import Any, Dict, Iterable, List, Optional, Set # noqa
from http import HTTPStatus
from typing import Any, Dict, Iterable, List, Optional, Set, Type, cast # noqa

from multidict import CIMultiDict
from yarl import URL

from . import hdrs
from .typedefs import LooseHeaders, StrOrURL
from .web_response import Response

__all__ = (
'HTTPException',
Expand Down Expand Up @@ -69,36 +73,77 @@
# HTTP Exceptions
############################################################

class HTTPException(Response, Exception):
class HTTPException(Exception):

# You should set in subclasses:
# status = 200

status_code = -1
empty_body = False
default_reason = "" # Initialized at the end of the module

__http_exception__ = True

def __init__(self, *,
headers: Optional[LooseHeaders]=None,
reason: Optional[str]=None,
body: Any=None,
text: Optional[str]=None,
content_type: Optional[str]=None) -> None:
if body is not None:
warnings.warn(
"body argument is deprecated for http web exceptions",
DeprecationWarning)
Response.__init__(self, status=self.status_code,
headers=headers, reason=reason,
body=body, text=text, content_type=content_type)
Exception.__init__(self, self.reason)
if self.body is None and not self.empty_body:
self.text = "{}: {}".format(self.status, self.reason)
if reason is None:
reason = self.default_reason

if text is None:
if not self.empty_body:
text = "{}: {}".format(self.status_code, reason)
else:
if self.empty_body:
warnings.warn(
kxepal marked this conversation as resolved.
Show resolved Hide resolved
"text argument is deprecated for HTTP status {} "
"in aiohttp 4.0 (#3462),"
"the response should be provided without a body".format(
self.status_code),
DeprecationWarning,
stacklevel=2)

if headers is not None:
real_headers = CIMultiDict(headers)
else:
real_headers = CIMultiDict()

if content_type is not None:
if not text:
warnings.warn("content_type without text is deprecated "
"in aiohttp 4.0 (#3462)",
DeprecationWarning,
stacklevel=2)
real_headers[hdrs.CONTENT_TYPE] = content_type
elif hdrs.CONTENT_TYPE not in real_headers and text:
real_headers[hdrs.CONTENT_TYPE] = 'text/plain'

super().__init__(reason)

self._text = text
self._headers = real_headers

def __bool__(self) -> bool:
return True

@property
def status(self) -> int:
return self.status_code

@property
def reason(self) -> str:
return self.args[0]

@property
def text(self) -> Optional[str]:
return self._text

@property
def headers(self) -> 'CIMultiDict[str]':
return self._headers


class HTTPError(HTTPException):
"""Base class for exceptions with status codes in the 400s and 500s."""
Expand Down Expand Up @@ -147,39 +192,42 @@ class HTTPPartialContent(HTTPSuccessful):
############################################################


class _HTTPMove(HTTPRedirection):
class HTTPMove(HTTPRedirection):

def __init__(self,
location: StrOrURL,
*,
headers: Optional[LooseHeaders]=None,
reason: Optional[str]=None,
body: Any=None,
text: Optional[str]=None,
content_type: Optional[str]=None) -> None:
if not location:
raise ValueError("HTTP redirects need a location to redirect to.")
super().__init__(headers=headers, reason=reason,
body=body, text=text, content_type=content_type)
text=text, content_type=content_type)
self.headers['Location'] = str(location)
self.location = location
self._location = URL(location)

@property
def location(self) -> URL:
return self._location


class HTTPMultipleChoices(_HTTPMove):
class HTTPMultipleChoices(HTTPMove):
status_code = 300


class HTTPMovedPermanently(_HTTPMove):
class HTTPMovedPermanently(HTTPMove):
status_code = 301


class HTTPFound(_HTTPMove):
class HTTPFound(HTTPMove):
status_code = 302


# This one is safe after a POST (the redirected location will be
# retrieved with GET):
class HTTPSeeOther(_HTTPMove):
class HTTPSeeOther(HTTPMove):
status_code = 303


Expand All @@ -189,16 +237,16 @@ class HTTPNotModified(HTTPRedirection):
empty_body = True


class HTTPUseProxy(_HTTPMove):
class HTTPUseProxy(HTTPMove):
# Not a move, but looks a little like one
status_code = 305


class HTTPTemporaryRedirect(_HTTPMove):
class HTTPTemporaryRedirect(HTTPMove):
status_code = 307


class HTTPPermanentRedirect(_HTTPMove):
class HTTPPermanentRedirect(HTTPMove):
status_code = 308


Expand Down Expand Up @@ -240,15 +288,22 @@ def __init__(self,
*,
headers: Optional[LooseHeaders]=None,
reason: Optional[str]=None,
body: Any=None,
text: Optional[str]=None,
content_type: Optional[str]=None) -> None:
allow = ','.join(sorted(allowed_methods))
super().__init__(headers=headers, reason=reason,
body=body, text=text, content_type=content_type)
text=text, content_type=content_type)
self.headers['Allow'] = allow
self.allowed_methods = set(allowed_methods) # type: Set[str]
self.method = method.upper()
self._allowed = set(allowed_methods) # type: Set[str]
Copy link
Member

Choose a reason for hiding this comment

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

Should allowed_methods be uppercased as well?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good question.
HTTP verbs are case sensitive by RFC.
Maybe even method.upper() is redundant.
aiohttp dispatcher never raises the lowercased method.

Copy link
Member

Choose a reason for hiding this comment

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

True. I'd say that method must never be transformed on the low level of the framework, normalization may be optional for dispatching calls to class-based views.
In Cheroot I return 400 Bad Request immediately as per RFCs: https://github.com/cherrypy/cheroot/blob/cf738c4/cheroot/server.py#L792-L802

self._method = method

@property
def allowed_methods(self) -> Set[str]:
return self._allowed

@property
def method(self) -> str:
return self._method


class HTTPNotAcceptable(HTTPClientError):
Expand Down Expand Up @@ -283,8 +338,8 @@ class HTTPRequestEntityTooLarge(HTTPClientError):
status_code = 413

def __init__(self,
max_size: float,
actual_size: float,
max_size: int,
actual_size: int,
**kwargs: Any) -> None:
kwargs.setdefault(
'text',
Expand Down Expand Up @@ -342,17 +397,20 @@ class HTTPUnavailableForLegalReasons(HTTPClientError):
status_code = 451

def __init__(self,
link: str,
link: StrOrURL,
*,
headers: Optional[LooseHeaders]=None,
reason: Optional[str]=None,
body: Any=None,
text: Optional[str]=None,
content_type: Optional[str]=None) -> None:
super().__init__(headers=headers, reason=reason,
body=body, text=text, content_type=content_type)
self.headers['Link'] = '<%s>; rel="blocked-by"' % link
self.link = link
text=text, content_type=content_type)
self.headers['Link'] = '<{}>; rel="blocked-by"'.format(str(link))
self._link = URL(link)

@property
def link(self) -> URL:
return self._link


############################################################
Expand Down Expand Up @@ -409,3 +467,19 @@ class HTTPNotExtended(HTTPServerError):

class HTTPNetworkAuthenticationRequired(HTTPServerError):
status_code = 511


def _initialize_default_reason() -> None:
for obj in globals().values():
asvetlov marked this conversation as resolved.
Show resolved Hide resolved
if isinstance(obj, type) and issubclass(obj, HTTPException):
asvetlov marked this conversation as resolved.
Show resolved Hide resolved
exc = cast(Type[HTTPException], obj)
if exc.status_code >= 0:
try:
status = HTTPStatus(exc.status_code)
exc.default_reason = status.phrase
except ValueError:
pass


_initialize_default_reason()
del _initialize_default_reason
4 changes: 2 additions & 2 deletions aiohttp/web_middlewares.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import re
from typing import TYPE_CHECKING, Awaitable, Callable, Tuple, Type, TypeVar

from .web_exceptions import HTTPMovedPermanently, _HTTPMove
from .web_exceptions import HTTPMove, HTTPMovedPermanently
from .web_request import Request
from .web_response import StreamResponse
from .web_urldispatcher import SystemRoute
Expand Down Expand Up @@ -42,7 +42,7 @@ def middleware(f: _Func) -> _Func:
def normalize_path_middleware(
*, append_slash: bool=True, remove_slash: bool=False,
merge_slashes: bool=True,
redirect_class: Type[_HTTPMove]=HTTPMovedPermanently) -> _Middleware:
redirect_class: Type[HTTPMove]=HTTPMovedPermanently) -> _Middleware:
"""
Middleware factory which produces a middleware that normalizes
the path of a request. By normalizing it means:
Expand Down
5 changes: 4 additions & 1 deletion aiohttp/web_protocol.py
Original file line number Diff line number Diff line change
Expand Up @@ -417,7 +417,10 @@ async def start(self) -> None:
self._request_handler(request))
resp = await task
except HTTPException as exc:
resp = exc
resp = Response(status=exc.status,
reason=exc.reason,
text=exc.text,
headers=exc.headers)
except asyncio.CancelledError:
self.log_debug('Ignored premature client disconnection')
break
Expand Down
1 change: 1 addition & 0 deletions docs/web.rst
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ The page contains all information about aiohttp Server API:
Advanced Usage <web_advanced>
Low Level <web_lowlevel>
Reference <web_reference>
Web Exceptions <web_exceptions>
Logging <logging>
Testing <testing>
Deployment <deployment>
Loading