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

feat(errors): default error serializer content negotiation #2190

Merged
Merged
Show file tree
Hide file tree
Changes from 11 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
4 changes: 4 additions & 0 deletions docs/_newsfragments/2023.newandimproved.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
The default error serializer will now use the response media handlers
to better negotiate the response serialization with the client.
The implementation still default to JSON in case a client does not specify
any preference.
3 changes: 3 additions & 0 deletions docs/_newsfragments/content_type.newandimproved.rst
CaselIT marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Added :attr:`falcon.testing.Result.content_type` and
:attr:`falcon.testing.StreamedResult.content_type` as an utility accessor
for the ``Content-Type`` header.
21 changes: 17 additions & 4 deletions falcon/app_helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,6 @@
exception: Instance of ``falcon.HTTPError``
"""
preferred = req.client_prefers((MEDIA_XML, 'text/xml', MEDIA_JSON))

CaselIT marked this conversation as resolved.
Show resolved Hide resolved
if preferred is None:
# NOTE(kgriffs): See if the client expects a custom media
# type based on something Falcon supports. Returning something
Expand All @@ -311,13 +310,27 @@
preferred = MEDIA_JSON
elif '+xml' in accept:
preferred = MEDIA_XML
else:
# NOTE(caselit): if nothing else matchers try using the media handlers
# registered in the response
preferred = req.client_prefers(list(resp.options.media_handlers))

Check warning on line 316 in falcon/app_helpers.py

View check run for this annotation

Codecov / codecov/patch

falcon/app_helpers.py#L316

Added line #L316 was not covered by tests
CaselIT marked this conversation as resolved.
Show resolved Hide resolved

if preferred is not None:
handler, serialize_sync, _ = resp.options.media_handlers._resolve(

Check warning on line 319 in falcon/app_helpers.py

View check run for this annotation

Codecov / codecov/patch

falcon/app_helpers.py#L319

Added line #L319 was not covered by tests
preferred, MEDIA_JSON, raise_not_found=False
)
if preferred == MEDIA_JSON:
handler, _, _ = resp.options.media_handlers._resolve(
MEDIA_JSON, MEDIA_JSON, raise_not_found=False
)
# NOTE(caselit): special case json to ensure that it's always possible to
# serialize an error in json even if no handler is set in the
# media_handlers.
resp.data = exception.to_json(handler)
elif handler:
CaselIT marked this conversation as resolved.
Show resolved Hide resolved
if serialize_sync:
CaselIT marked this conversation as resolved.
Show resolved Hide resolved
resp.data = serialize_sync(exception.to_dict(), preferred)

Check warning on line 329 in falcon/app_helpers.py

View check run for this annotation

Codecov / codecov/patch

falcon/app_helpers.py#L329

Added line #L329 was not covered by tests
else:
# NOTE(caselit): Let the app serialize the response if there is no sync
# serializer implemented in the handler.
resp.media = exception.to_dict()

Check warning on line 333 in falcon/app_helpers.py

View check run for this annotation

Codecov / codecov/patch

falcon/app_helpers.py#L333

Added line #L333 was not covered by tests
else:
resp.data = exception.to_xml()

Expand Down
13 changes: 7 additions & 6 deletions falcon/http_error.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@

from __future__ import annotations

from collections import OrderedDict
from typing import MutableMapping, Optional, Type, TYPE_CHECKING, Union
import xml.etree.ElementTree as et

Expand Down Expand Up @@ -144,10 +143,11 @@
self.code = code

if href:
link = self.link = OrderedDict()
link['text'] = href_text or 'Documentation related to this error'
link['href'] = uri.encode(href)
link['rel'] = 'help'
self.link = {

Check warning on line 146 in falcon/http_error.py

View check run for this annotation

Codecov / codecov/patch

falcon/http_error.py#L146

Added line #L146 was not covered by tests
'text': href_text or 'Documentation related to this error',
'href': uri.encode(href),
'rel': 'help',
}
else:
self.link = None

Expand Down Expand Up @@ -209,9 +209,10 @@

"""

obj = self.to_dict(OrderedDict)
obj = self.to_dict()

Check warning on line 212 in falcon/http_error.py

View check run for this annotation

Codecov / codecov/patch

falcon/http_error.py#L212

Added line #L212 was not covered by tests
if handler is None:
handler = _DEFAULT_JSON_HANDLER
# NOTE: the json handler requires the sync serialize interface
return handler.serialize(obj, MEDIA_JSON)

def to_xml(self) -> bytes:
Expand Down
7 changes: 6 additions & 1 deletion falcon/testing/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -274,6 +274,11 @@
"""
return self._encoding

@property
def content_type(self) -> Optional[str]:
CaselIT marked this conversation as resolved.
Show resolved Hide resolved
"""Return the ``Content-Type`` header or ``None`` if missing."""
return self.headers.get('Content-Type')

Check warning on line 280 in falcon/testing/client.py

View check run for this annotation

Codecov / codecov/patch

falcon/testing/client.py#L280

Added line #L280 was not covered by tests


class ResultBodyStream:
"""Simple forward-only reader for a streamed test result body.
Expand Down Expand Up @@ -369,7 +374,7 @@
return json_module.loads(self.text)

def __repr__(self) -> str:
content_type = self.headers.get('Content-Type', '')
content_type = self.content_type or ''

Check warning on line 377 in falcon/testing/client.py

View check run for this annotation

Codecov / codecov/patch

falcon/testing/client.py#L377

Added line #L377 was not covered by tests

if len(self.content) > 40:
content = self.content[:20] + b'...' + self.content[-20:]
Expand Down
99 changes: 96 additions & 3 deletions tests/test_httperror.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@
import pytest

import falcon
from falcon.constants import MEDIA_JSON
from falcon.constants import MEDIA_XML
from falcon.constants import MEDIA_YAML
from falcon.media import BaseHandler
import falcon.testing as testing

try:
Expand Down Expand Up @@ -943,7 +947,96 @@ def test_MediaMalformedError(self):
err.__cause__ = ValueError('some error')
assert err.description == 'Could not parse foo-media body - some error'

def test_kw_only(self):
with pytest.raises(TypeError, match='positional argument'):
falcon.HTTPError(falcon.HTTP_BAD_REQUEST, 'foo', 'bar')

def test_kw_only():
with pytest.raises(TypeError, match='positional argument'):
falcon.HTTPError(falcon.HTTP_BAD_REQUEST, 'foo', 'bar')

JSON_CONTENT = b'{"title": "410 Gone"}'
JSON = (MEDIA_JSON, MEDIA_JSON, JSON_CONTENT)
CUSTOM_JSON = ('custom/any+json', MEDIA_JSON, JSON_CONTENT)

XML_CONTENT = (
b'<?xml version="1.0" encoding="UTF-8"?><error><title>410 Gone</title></error>'
)
XML = (MEDIA_XML, MEDIA_XML, XML_CONTENT)
CUSTOM_XML = ('custom/any+xml', MEDIA_XML, XML_CONTENT)

YAML = (MEDIA_YAML, MEDIA_YAML, (b'title: 410 Gone!'))
ASYNC_ONLY = ('application/only_async', 'application/only_async', b'this is async')
ASYNC_WITH_SYNC = (
'application/async_with_sync',
'application/async_with_sync',
b'this is sync instead',
)


class FakeYamlMediaHandler(BaseHandler):
def serialize(self, media: object, content_type: str) -> bytes:
assert media == {'title': '410 Gone'}
return b'title: 410 Gone!'


class AsyncOnlyMediaHandler(BaseHandler):
async def serialize_async(self, media: object, content_type: str) -> bytes:
assert media == {'title': '410 Gone'}
return b'this is async'


class SyncInterfaceMediaHandler(AsyncOnlyMediaHandler):
def serialize(self, media: object, content_type: str) -> bytes:
assert media == {'title': '410 Gone'}
return b'this is sync instead'

_serialize_sync = serialize


class TestDefaultSerializeError:
@pytest.fixture
def client(self, util, asgi):
app = util.create_app(asgi)
app.add_route('/', GoneResource())
return testing.TestClient(app)

@pytest.mark.parametrize('has_json_handler', [True, False])
def test_defaults_to_json(self, client, has_json_handler):
if not has_json_handler:
client.app.req_options.media_handlers.pop(MEDIA_JSON)
client.app.resp_options.media_handlers.pop(MEDIA_JSON)
res = client.simulate_get()
assert res.content_type == 'application/json'
assert res.headers['vary'] == 'Accept'
assert res.content == JSON_CONTENT

@pytest.mark.parametrize(
'accept, content_type, content',
(JSON, XML, CUSTOM_JSON, CUSTOM_XML, YAML, ASYNC_ONLY, ASYNC_WITH_SYNC),
)
def test_serializes_error_to_preferred_by_sender(
self, accept, content_type, content, client, asgi
):
client.app.resp_options.media_handlers[MEDIA_YAML] = FakeYamlMediaHandler()
client.app.resp_options.media_handlers[ASYNC_WITH_SYNC[0]] = (
SyncInterfaceMediaHandler()
)
if asgi:
client.app.resp_options.media_handlers[ASYNC_ONLY[0]] = (
AsyncOnlyMediaHandler()
)
res = client.simulate_get(headers={'Accept': accept})
assert res.headers['vary'] == 'Accept'
if content_type == ASYNC_ONLY[0] and not asgi:
# media-json is the default content type
assert res.content_type == MEDIA_JSON
assert res.content == b''
else:
assert res.content_type == content_type
assert res.content == content

def test_json_async_only_error(self, util):
app = util.create_app(True)
app.add_route('/', GoneResource())
app.resp_options.media_handlers[MEDIA_JSON] = AsyncOnlyMediaHandler()
client = testing.TestClient(app)
with pytest.raises(NotImplementedError, match='requires the sync interface'):
client.simulate_get()
12 changes: 12 additions & 0 deletions tests/test_media_handlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
from falcon import media
from falcon import testing
from falcon.asgi.stream import BoundedStream
from falcon.constants import MEDIA_JSON
from falcon.constants import MEDIA_YAML

mujson = None
orjson = None
Expand Down Expand Up @@ -372,6 +374,16 @@ def on_get(self, req, resp):
assert result.json == falcon.HTTPForbidden().to_dict()


def test_handlers_include_new_media_handlers_in_resolving() -> None:
handlers = media.Handlers({falcon.MEDIA_URLENCODED: media.URLEncodedFormHandler()})
assert handlers._resolve(MEDIA_YAML, MEDIA_JSON, raise_not_found=False)[0] is None

js_handler = media.JSONHandler()
handlers[MEDIA_YAML] = js_handler
resolved = handlers._resolve(MEDIA_YAML, MEDIA_JSON, raise_not_found=False)[0]
assert resolved is js_handler


class TestBaseHandler:
def test_defaultError(self):
h = media.BaseHandler()
Expand Down
27 changes: 27 additions & 0 deletions tests/test_testing.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
from falcon import App
from falcon import status_codes
from falcon import testing
from falcon.util.sync import async_to_sync


class CustomCookies:
Expand Down Expand Up @@ -103,6 +104,32 @@ def on_post(self, req, resp):
assert result.text == falcon.MEDIA_JSON


@pytest.mark.parametrize('mode', ['wsgi', 'asgi', 'asgi-stream'])
def test_content_type(util, mode):
class Responder:
def on_get(self, req, resp):
resp.content_type = req.content_type

app = util.create_app('asgi' in mode)
app.add_route('/', Responder())

if 'stream' in mode:

async def go():
async with testing.ASGIConductor(app) as ac:
async with ac.simulate_get_stream(
'/', content_type='my-content-type'
) as r:
assert r.content_type == 'my-content-type'
return 1

assert async_to_sync(go) == 1
else:
client = testing.TestClient(app)
res = client.simulate_get('/', content_type='foo-content')
assert res.content_type == 'foo-content'


@pytest.mark.parametrize('cookies', [{'foo': 'bar', 'baz': 'foo'}, CustomCookies()])
def test_create_environ_cookies(cookies):
environ = testing.create_environ(cookies=cookies)
Expand Down
Loading