Fix Honeybadger error reporting

Closes: #107

CONTRIBUTING.rst

@@ -82,6 +82,7 @@ Ready to contribute? Here's how to set up `apphelpers` for local development.
 
 
    $ export SETTINGS_DIR=.
+   $ docker-compose up -d  # start postgres and redis
    $ gunicorn tests.service:__hug_wsgi__
    $ pytest tests
 
@@ -90,6 +91,7 @@ Ready to contribute? Here's how to set up `apphelpers` for local development.
 
 
    $ export SETTINGS_DIR=.
+   $ docker-compose up -d  # start postgres and redis
    $ uvicorn fastapi_tests.service:app --host 0.0.0.0 --port 5000
    $ pytest fastapi_tests
 

apphelpers/rest/common.py

@@ -1,15 +1,13 @@
 from __future__ import annotations
 
-from dataclasses import (
-    asdict,
-    dataclass,
-    field,
-)
-from typing import (
-    Dict,
-    List,
-    Optional,
-)
+from dataclasses import asdict, dataclass, field
+from typing import Dict, List, Optional
+
+from converge import settings
+from requests.exceptions import HTTPError
+
+if settings.get("HONEYBADGER_API_KEY"):
+    from honeybadger.utils import filter_dict
 
 
 def phony(f):
@@ -32,3 +30,19 @@ def to_dict(self):
 
     def __bool__(self):
         return self.id is not None
+
+
+def notify_honeybadger(honeybadger, error, func, args, kwargs):
+    try:
+        honeybadger.notify(
+            error,
+            context={
+                "func": func.__name__,
+                "args": args,
+                "kwargs": filter_dict(kwargs, settings.HB_PARAM_FILTERS),
+            },
+        )
+    except HTTPError as e:
+        if e.response.status_code != 403:
+            # Ignore 403 Forbidden errors. We get alerted by HB anyway.
+            raise e

apphelpers/rest/fastapi.py

@@ -8,18 +8,18 @@
 
 from apphelpers.db import dbtransaction_ctx
 from apphelpers.errors.fastapi import (
+    BaseError,
     HTTP401Unauthorized,
     HTTP403Forbidden,
     HTTP404NotFound,
     InvalidSessionError,
 )
 from apphelpers.rest import endpoint as ep
-from apphelpers.rest.common import User, phony
+from apphelpers.rest.common import User, notify_honeybadger, phony
 from apphelpers.sessions import SessionDBHandler
 
 if settings.get("HONEYBADGER_API_KEY"):
     from honeybadger import Honeybadger
-    from honeybadger.utils import filter_dict
 
 
 def raise_not_found_on_none(f):
@@ -54,25 +54,45 @@ def honeybadger_wrapper(hb):
     """
 
     def wrapper(f):
-        @wraps(f)
-        def f_wrapped(*args, **kw):
-            try:
-                ret = f(*args, **kw)
-            except Exception as e:
+        if inspect.iscoroutinefunction(f):
+
+            @wraps(f)
+            async def async_f_wrapped(*args, **kw):
                 try:
-                    hb.notify(
-                        e,
-                        context={
-                            "func": f.__name__,
-                            "args": args,
-                            "kwargs": filter_dict(kw, settings.HB_PARAM_FILTERS),
-                        },
+                    return await f(*args, **kw)
+                except BaseError as e:
+                    if e.report:
+                        notify_honeybadger(
+                            honeybadger=hb, error=e, func=f, args=args, kwargs=kw
+                        )
+                    raise e
+                except Exception as e:
+                    notify_honeybadger(
+                        honeybadger=hb, error=e, func=f, args=args, kwargs=kw
+                    )
+                    raise e
+
+            return async_f_wrapped
+
+        else:
+
+            @wraps(f)
+            def f_wrapped(*args, **kw):
+                try:
+                    return f(*args, **kw)
+                except BaseError as e:
+                    if e.report:
+                        notify_honeybadger(
+                            honeybadger=hb, error=e, func=f, args=args, kwargs=kw
+                        )
+                    raise e
+                except Exception as e:
+                    notify_honeybadger(
+                        honeybadger=hb, error=e, func=f, args=args, kwargs=kw
                     )
-                finally:
                     raise e
-            return ret
 
-        return f_wrapped
+            return f_wrapped
 
     return wrapper
 

apphelpers/rest/hug.py

@@ -11,11 +11,11 @@
 from apphelpers.errors.hug import BaseError, InvalidSessionError
 from apphelpers.loggers import api_logger
 from apphelpers.rest import endpoint as ep
+from apphelpers.rest.common import notify_honeybadger
 from apphelpers.sessions import SessionDBHandler
 
 if settings.get("HONEYBADGER_API_KEY"):
     from honeybadger import Honeybadger
-    from honeybadger.utils import filter_dict
 
 
 def phony(f):
@@ -36,20 +36,6 @@ def wrapper(*ar, **kw):
     return f
 
 
-def notify_honeybadger(honeybadger, error, func, args, kwargs):
-    try:
-        honeybadger.notify(
-            error,
-            context={
-                "func": func.__name__,
-                "args": args,
-                "kwargs": filter_dict(kwargs, settings.HB_PARAM_FILTERS),
-            },
-        )
-    finally:
-        pass
-
-
 def honeybadger_wrapper(hb):
     """
     wrapper that executes the function in a try/except

default_settings.py

@@ -15,6 +15,9 @@
 SMTP_USERNAME = None
 SMTP_KEY = ""
 
+HONEYBADGER_API_KEY = "secret"
+HB_PARAM_FILTERS = ["password", "passwd", "secret"]
+
 
 class API_LOGGER:
     ENABLED = False

docker-compose.yml

@@ -0,0 +1,15 @@
+version: "3.1"
+services:
+  postgres:
+    image: postgres
+    ports:
+      - 5432:5432
+    environment:
+      POSTGRES_DB: defaultdb
+      POSTGRES_USER: postgres
+      POSTGRES_PASSWORD: postgres
+
+  redis:
+    image: redis
+    ports:
+      - 6379:6379

fastapi_tests/test_rest.py

@@ -1,8 +1,14 @@
+import asyncio
+from unittest import mock
+
+import pytest
 import requests
 from converge import settings
 
 import apphelpers.sessions as sessionslib
-from apphelpers.db.piccolo import setup_db_from_basetable, destroy_db_from_basetable
+from apphelpers.db.piccolo import destroy_db_from_basetable, setup_db_from_basetable
+from apphelpers.errors.fastapi import BaseError
+from apphelpers.rest.fastapi import honeybadger_wrapper
 from fastapi_tests.app.models import BaseTable
 
 base_url = "http://127.0.0.1:5000/"
@@ -312,3 +318,52 @@ def test_piccolo():
 
     url = base_url + "count-books"
     assert requests.get(url).json() == 3
+
+
+def test_honeybadger_wrapper():
+
+    mocked_honeybadger = mock.MagicMock()
+    wrapper = honeybadger_wrapper(mocked_honeybadger)
+
+    def good_endpoint(foo):
+        return foo
+
+    class IgnorableError(BaseError):
+        report = False
+
+    async def bad_endpoint(foo):
+        raise IgnorableError()
+
+    async def worse_endpoint(foo, password):
+        raise BaseError()
+
+    async def worst_endpoint(foo):
+        raise RuntimeError()
+
+    wrapped_good_endpoint = wrapper(good_endpoint)
+    wrapped_bad_endpoint = wrapper(bad_endpoint)
+    wrapped_worse_endpoint = wrapper(worse_endpoint)
+    wrapped_worst_endpoint = wrapper(worst_endpoint)
+
+    assert wrapped_good_endpoint(1) == 1
+    assert not mocked_honeybadger.notify.called
+
+    with pytest.raises(IgnorableError):
+        asyncio.run(wrapped_bad_endpoint(1))
+    assert not mocked_honeybadger.notify.called
+
+    with pytest.raises(BaseError) as e:
+        asyncio.run(wrapped_worse_endpoint(1, password="secret"))
+    mocked_honeybadger.notify.assert_called_once_with(
+        e.value,
+        context={
+            "func": "worse_endpoint",
+            "args": (1,),
+            "kwargs": {"password": "[FILTERED]"},
+        },
+    )
+    assert mocked_honeybadger.notify.call_count == 1
+
+    with pytest.raises(RuntimeError):
+        asyncio.run(wrapped_worst_endpoint(1))
+    assert mocked_honeybadger.notify.call_count == 2

requirements_dev.txt

@@ -30,3 +30,4 @@ types-redis
 redis
 loguru
 piccolo[postgres]
+honeybadger

tests/app/endpoints.py

@@ -1,8 +1,5 @@
-from typing import Dict, Optional
-
 import hug
 import hug.directives
-from pydantic import BaseModel
 
 from apphelpers.rest import endpoint as ep
 from apphelpers.rest.hug import user_id

tests/test_rest.py

@@ -1,10 +1,14 @@
 import os
 import time
+from unittest import mock
 
+import pytest
 import requests
 from converge import settings
 
 import apphelpers.sessions as sessionslib
+from apphelpers.errors.hug import BaseError
+from apphelpers.rest.hug import honeybadger_wrapper
 
 from .app.models import globalgroups, sitegroups
 
@@ -311,3 +315,52 @@ def test_custom_authorization_access():
 
     url = urls.echo_for_custom_authorization + "/unauthorized"
     assert requests.get(url, headers=headers).status_code == 403
+
+
+def test_honeybadger_wrapper():
+
+    mocked_honeybadger = mock.MagicMock()
+    wrapper = honeybadger_wrapper(mocked_honeybadger)
+
+    def good_endpoint(foo):
+        return foo
+
+    class IgnorableError(BaseError):
+        report = False
+
+    def bad_endpoint(foo):
+        raise IgnorableError()
+
+    def worse_endpoint(foo, password):
+        raise BaseError()
+
+    def worst_endpoint(foo):
+        raise RuntimeError()
+
+    wrapped_good_endpoint = wrapper(good_endpoint)
+    wrapped_bad_endpoint = wrapper(bad_endpoint)
+    wrapped_worse_endpoint = wrapper(worse_endpoint)
+    wrapped_worst_endpoint = wrapper(worst_endpoint)
+
+    assert wrapped_good_endpoint(1) == 1
+    assert not mocked_honeybadger.notify.called
+
+    with pytest.raises(IgnorableError):
+        wrapped_bad_endpoint(1)
+    assert not mocked_honeybadger.notify.called
+
+    with pytest.raises(BaseError) as e:
+        wrapped_worse_endpoint(1, password="secret")
+    mocked_honeybadger.notify.assert_called_once_with(
+        e.value,
+        context={
+            "func": "worse_endpoint",
+            "args": (1,),
+            "kwargs": {"password": "[FILTERED]"},
+        },
+    )
+    assert mocked_honeybadger.notify.call_count == 1
+
+    with pytest.raises(RuntimeError):
+        wrapped_worst_endpoint(1)
+    assert mocked_honeybadger.notify.call_count == 2