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

add exc_info to access_logger #3557

Merged
merged 8 commits into from
May 13, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions CHANGES/3557.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Call ``AccessLogger.log`` with the current exception available from sys.exc_info().
64 changes: 41 additions & 23 deletions aiohttp/web_protocol.py
Original file line number Diff line number Diff line change
Expand Up @@ -342,9 +342,10 @@ def force_close(self) -> None:
def log_access(self,
request: BaseRequest,
response: StreamResponse,
time: float) -> None:
request_start: float) -> None:
if self.access_logger is not None:
self.access_logger.log(request, response, time)
self.access_logger.log(request, response,
self._loop.time() - request_start)

def log_debug(self, *args: Any, **kw: Any) -> None:
if self.debug:
Expand Down Expand Up @@ -402,8 +403,7 @@ async def start(self) -> None:

message, payload = self._messages.popleft()

if self.access_log:
now = loop.time()
start = loop.time()

manager.requests_count += 1
writer = StreamWriter(self, loop)
Expand All @@ -420,41 +420,29 @@ async def start(self) -> None:
reason=exc.reason,
text=exc.text,
headers=exc.headers)
reset = await self.finish_response(request, resp, start)
except asyncio.CancelledError:
self.log_debug('Ignored premature client disconnection')
break
except asyncio.TimeoutError as exc:
self.log_debug('Request handler timed out.', exc_info=exc)
resp = self.handle_error(request, 504)
reset = await self.finish_response(request, resp, start)
except Exception as exc:
resp = self.handle_error(request, 500, exc)
reset = await self.finish_response(request, resp, start)
else:
reset = await self.finish_response(request, resp, start)

# Drop the processed task from asyncio.Task.all_tasks() early
del task

try:
prepare_meth = resp.prepare
except AttributeError:
if resp is None:
raise RuntimeError("Missing return "
"statement on request handler")
else:
raise RuntimeError("Web-handler should return "
"a response instance, "
"got {!r}".format(resp))
try:
await prepare_meth(request)
await resp.write_eof()
except ConnectionResetError:
if reset:
self.log_debug('Ignored premature client disconnection 2')
break

# notify server about keep-alive
self._keepalive = bool(resp.keep_alive)

# log access
if self.access_log:
self.log_access(request, resp, loop.time() - now)

# check payload
if not payload.is_eof():
lingering_time = self._lingering_time
Expand Down Expand Up @@ -514,6 +502,36 @@ async def start(self) -> None:
if self.transport is not None and self._error_handler is None:
self.transport.close()

async def finish_response(self,
request: BaseRequest,
resp: StreamResponse,
start_time: float) -> bool:
"""
Prepare the response and write_eof, then log access. This has to
be called within the context of any exception so the access logger
can get exception information. Returns True if the client disconnects
prematurely.
"""
try:
prepare_meth = resp.prepare
except AttributeError:
if resp is None:
raise RuntimeError("Missing return "
"statement on request handler")
else:
raise RuntimeError("Web-handler should return "
"a response instance, "
"got {!r}".format(resp))
try:
await prepare_meth(request)
await resp.write_eof()
except ConnectionResetError:
self.log_access(request, resp, start_time)
return True
else:
self.log_access(request, resp, start_time)
return False

def handle_error(self,
request: BaseRequest,
status: int=500,
Expand Down
9 changes: 6 additions & 3 deletions docs/logging.rst
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,12 @@ Example of a drop-in replacement for :class:`aiohttp.helpers.AccessLogger`::
f'done in {time}s: {response.status}')


.. versionadded:: 4.0.0


``AccessLogger.log()`` can now access any exception raised while processing
the request with ``sys.exc_info()``.

.. _gunicorn-accesslog:

Gunicorn access logs
Expand All @@ -123,9 +129,6 @@ a custom logging setup intercepting the ``'gunicorn.access'`` logger,
accesslog_ should be set to ``'-'`` to prevent Gunicorn to create an empty
access log file upon every startup.




Error logs
----------

Expand Down
22 changes: 22 additions & 0 deletions tests/test_web_log.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import datetime
import platform
import sys
from unittest import mock

import pytest
Expand Down Expand Up @@ -156,3 +157,24 @@ def log(self, request, response, time):
access_logger = Logger(mock_logger, '{request} {response} {time}')
access_logger.log('request', 'response', 1)
mock_logger.info.assert_called_with('request response 1')


async def test_exc_info_context(aiohttp_raw_server, aiohttp_client):
exc_msg = None

class Logger(AbstractAccessLogger):
def log(self, request, response, time):
nonlocal exc_msg
exc_msg = '{0.__name__}: {1}'.format(*sys.exc_info())

async def handler(request):
raise RuntimeError('intentional runtime error')

logger = mock.Mock()
server = await aiohttp_raw_server(handler,
access_log_class=Logger,
logger=logger)
cli = await aiohttp_client(server)
resp = await cli.get('/path/to', headers={'Accept': 'text/html'})
assert resp.status == 500
assert exc_msg == 'RuntimeError: intentional runtime error'