From 2b8e8816ad613b024c73bc20dad777a41b7febd3 Mon Sep 17 00:00:00 2001 From: Julien Danjou Date: Fri, 20 Jan 2017 21:09:51 +0100 Subject: [PATCH 1/2] routing: stop printing warning if the user passes a wrong URL When a _lookup method accepts N + *remainder arguments but the number of path components (separated by /) is < N, a TypeError is raised when calling the object and Pecan prints a RuntimeWarning that is log. Unfortunately this just fills up the log with no interesting information for the developer as this is just a standard 404 error. --- pecan/routing.py | 10 +++++----- pecan/tests/test_base.py | 8 +++----- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/pecan/routing.py b/pecan/routing.py index aed8ad5..90a9126 100644 --- a/pecan/routing.py +++ b/pecan/routing.py @@ -1,3 +1,4 @@ +import logging import re import warnings from inspect import getmembers, ismethod @@ -12,6 +13,8 @@ __all__ = ['lookup_controller', 'find_object', 'route'] __observed_controllers__ = set() __custom_routes__ = {} +logger = logging.getLogger(__name__) + def route(*args): """ @@ -177,11 +180,8 @@ def handle_lookup_traversal(obj, args): cross_boundary(prev_obj, obj) return result except TypeError as te: - msg = 'Got exception calling lookup(): %s (%s)' - warnings.warn( - msg % (te, te.args), - RuntimeWarning - ) + logger.debug('Got exception calling lookup(): %s (%s)', + te, te.args) def find_object(obj, remainder, notfound_handlers, request): diff --git a/pecan/tests/test_base.py b/pecan/tests/test_base.py index 20a337b..847c757 100644 --- a/pecan/tests/test_base.py +++ b/pecan/tests/test_base.py @@ -339,11 +339,9 @@ class TestLookups(PecanTestCase): def _lookup(self, someID): return 'Bad arg spec' # pragma: nocover - with warnings.catch_warnings(): - warnings.simplefilter("ignore") - app = TestApp(Pecan(RootController())) - r = app.get('/foo/bar', expect_errors=True) - assert r.status_int == 404 + app = TestApp(Pecan(RootController())) + r = app.get('/foo/bar', expect_errors=True) + assert r.status_int == 404 class TestCanonicalLookups(PecanTestCase): From 93d1a08f5410b469fec6f60df75bf6e05f767c6c Mon Sep 17 00:00:00 2001 From: Julien Danjou Date: Sat, 21 Jan 2017 18:11:37 +0100 Subject: [PATCH 2/2] routing: do not catch TypeError too wildly The current scope if try/except it way larger than it needs to be, which could make it catch errors that are not tied to the method being called. --- pecan/routing.py | 7 ++++--- pecan/tests/test_base.py | 11 +++++++++++ 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/pecan/routing.py b/pecan/routing.py index 90a9126..9b2f57b 100644 --- a/pecan/routing.py +++ b/pecan/routing.py @@ -173,15 +173,16 @@ def lookup_controller(obj, remainder, request=None): def handle_lookup_traversal(obj, args): try: result = obj(*args) + except TypeError as te: + logger.debug('Got exception calling lookup(): %s (%s)', + te, te.args) + else: if result: prev_obj = obj obj, remainder = result # crossing controller boundary cross_boundary(prev_obj, obj) return result - except TypeError as te: - logger.debug('Got exception calling lookup(): %s (%s)', - te, te.args) def find_object(obj, remainder, notfound_handlers, request): diff --git a/pecan/tests/test_base.py b/pecan/tests/test_base.py index 847c757..0969755 100644 --- a/pecan/tests/test_base.py +++ b/pecan/tests/test_base.py @@ -343,6 +343,17 @@ class TestLookups(PecanTestCase): r = app.get('/foo/bar', expect_errors=True) assert r.status_int == 404 + def test_lookup_with_wrong_return(self): + class RootController(object): + @expose() + def _lookup(self, someID, *remainder): + return 1 + + app = TestApp(Pecan(RootController())) + self.assertRaises(TypeError, + app.get, + '/foo/bar', expect_errors=True) + class TestCanonicalLookups(PecanTestCase):