Fix handling of cherrypy HTTPRedirects (#324)

* Fix handling of cherrypy HTTPRedirects

* Format and linting

* Add more linting ignored rules

* Further linter tuning

Co-authored-by: Tim Pansino <[email protected]>

Author: bdeeney

.mega-linter.yml

@@ -21,6 +21,14 @@ ENABLE_LINTERS: # If you use ENABLE_LINTERS variable, all other linters will be
   - YAML_PRETTIER
   - YAML_V8R
   - YAML_YAMLLINT
+
+PYTHON_FLAKE8_CONFIG_FILE: setup.cfg
+PYTHON_BLACK_CONFIG_FILE: pyproject.toml
+PYTHON_PYLINT_CONFIG_FILE: pyproject.toml
+PYTHON_ISORT_CONFIG_FILE: pyproject.toml
+
+PYTHON_PYLINT_ARGUMENTS: "--fail-under=0 --fail-on=E"
+
 # IGNORE_GITIGNORED_FILES: true # Currently broken
 EXCLUDED_DIRECTORIES:
   - "__pycache__"

newrelic/hooks/framework_cherrypy.py

@@ -37,19 +37,23 @@
 # * We don't handle any sub dispatching that may be occurring due to the
 #   use of XMLRPCDispatcher.
 
+from newrelic.api.error_trace import wrap_error_trace
 from newrelic.api.function_trace import FunctionTrace, wrap_function_trace
-from newrelic.api.transaction import current_transaction
 from newrelic.api.time_trace import notice_error
+from newrelic.api.transaction import current_transaction
 from newrelic.api.wsgi_application import wrap_wsgi_application
 from newrelic.common.object_names import callable_name
-from newrelic.common.object_wrapper import (ObjectProxy, function_wrapper,
-        wrap_function_wrapper)
-from newrelic.api.error_trace import wrap_error_trace
+from newrelic.common.object_wrapper import (
+    ObjectProxy,
+    function_wrapper,
+    wrap_function_wrapper,
+)
 
 
 def framework_details():
     import cherrypy
-    return ('CherryPy', getattr(cherrypy, '__version__', None))
+
+    return ("CherryPy", getattr(cherrypy, "__version__", None))
 
 
 def status_code(exc, value, tb):
@@ -62,9 +66,11 @@ def status_code(exc, value, tb):
         # In the case of an HTTPError that is malformed (e.g.
         # HTTPError("10 Bad error")), value will not have a code attr.
         # In both of those cases, we fall back to value.status
-        code = getattr(value, 'code', value.status)
+        code = getattr(value, "code", value.status)
         return code
 
+    return None
+
 
 @function_wrapper
 def handler_wrapper(wrapped, instance, args, kwargs):
@@ -84,14 +90,13 @@ def handler_wrapper(wrapped, instance, args, kwargs):
 
 
 class ResourceProxy(ObjectProxy):
-
     def __getattr__(self, name):
         # Methods on the wrapped object corresponding to the HTTP
         # method will always be upper case. Wrap the method when
         # returned with the handler wrapper.
 
         attr = super(ResourceProxy, self).__getattr__(name)
-        return name.isupper() and handler_wrapper(attr) or attr
+        return handler_wrapper(attr) if name.isupper() else attr
 
 
 def wrapper_Dispatcher_find_handler(wrapped, instance, args, kwargs):
@@ -108,19 +113,19 @@ def wrapper_Dispatcher_find_handler(wrapped, instance, args, kwargs):
     except:  # Catch all
         # Can end up here when a custom _cp_dispatch() method is
         # used and that raises an exception.
-        notice_error()
+        notice_error(status_code=status_code)
         raise
 
     if obj:
-        if instance.__class__.__name__ == 'MethodDispatcher':
+        if instance.__class__.__name__ == "MethodDispatcher":
             # We initially name the web transaction as if the
             # corresponding method for the HTTP method will not
             # be found and the request will not be allowed. This
             # will be overridden with the actual handler name
             # when the subsequent wrapper around the handler is
             # executed.
 
-            transaction.set_transaction_name('405', group='StatusCode')
+            transaction.set_transaction_name("405", group="StatusCode")
 
             # We have to use a custom object proxy here in order
             # to intercept accesses made by the dispatcher on the
@@ -141,7 +146,7 @@ def wrapper_Dispatcher_find_handler(wrapped, instance, args, kwargs):
         # No handler could be found so name the web transaction
         # after the 404 status code.
 
-        transaction.set_transaction_name('404', group='StatusCode')
+        transaction.set_transaction_name("404", group="StatusCode")
 
     return obj, vpath
 
@@ -159,7 +164,7 @@ def wrapper_RoutesDispatcher_find_handler(wrapped, instance, args, kwargs):
 
     except:  # Catch all
         # Can end up here when the URL was invalid in some way.
-        notice_error()
+        notice_error(status_code=status_code)
         raise
 
     if handler:
@@ -172,34 +177,31 @@ def wrapper_RoutesDispatcher_find_handler(wrapped, instance, args, kwargs):
         # No handler could be found so name the web transaction
         # after the 404 status code.
 
-        transaction.set_transaction_name('404', group='StatusCode')
+        transaction.set_transaction_name("404", group="StatusCode")
 
     return handler
 
 
 def instrument_cherrypy__cpreqbody(module):
-    wrap_function_trace(module, 'process_multipart')
-    wrap_function_trace(module, 'process_multipart_form_data')
+    wrap_function_trace(module, "process_multipart")
+    wrap_function_trace(module, "process_multipart_form_data")
 
 
 def instrument_cherrypy__cprequest(module):
-    wrap_function_trace(module, 'Request.handle_error')
+    wrap_function_trace(module, "Request.handle_error")
 
 
 def instrument_cherrypy__cpdispatch(module):
-    wrap_function_wrapper(module, 'Dispatcher.find_handler',
-            wrapper_Dispatcher_find_handler)
-    wrap_function_wrapper(module, 'RoutesDispatcher.find_handler',
-            wrapper_RoutesDispatcher_find_handler)
-    wrap_error_trace(module, 'PageHandler.__call__',
-            ignore=['cherrypy._cperror:InternalRedirect'], status_code=status_code)
+    wrap_function_wrapper(module, "Dispatcher.find_handler", wrapper_Dispatcher_find_handler)
+    wrap_function_wrapper(module, "RoutesDispatcher.find_handler", wrapper_RoutesDispatcher_find_handler)
+    wrap_error_trace(
+        module, "PageHandler.__call__", ignore=["cherrypy._cperror:InternalRedirect"], status_code=status_code
+    )
 
 
 def instrument_cherrypy__cpwsgi(module):
-    wrap_wsgi_application(module, 'CPWSGIApp.__call__',
-            framework=framework_details())
+    wrap_wsgi_application(module, "CPWSGIApp.__call__", framework=framework_details())
 
 
 def instrument_cherrypy__cptree(module):
-    wrap_wsgi_application(module, 'Application.__call__',
-            framework=framework_details())
+    wrap_wsgi_application(module, "Application.__call__", framework=framework_details())

pyproject.toml

@@ -6,7 +6,10 @@ include = '\.pyi?$'
 profile = "black"
 
 [tool.pylint.messages_control]
-disable = "C0330, C0326"
+disable = "C0103,C0115,C0116,C0415,E0401,E1120,E122,E126,E127,E128,E203,E501,E722,R1725,W0613,W0613,W504"
 
 [tool.pylint.format]
-max-line-length = "88"
+max-line-length = "120"
+
+[tool.pylint.basic]
+good-names = "exc,val,tb"

setup.cfg

@@ -5,4 +5,4 @@ license_files =
 
 [flake8]
 max-line-length = 120
-extend-ignore = E203,E501
+extend-ignore = C0103,C0115,C0116,C0415,E0401,E1120,E122,E126,E127,E128,E203,E501,E722,R1725,W0613,W0613,W504