Fix Honeybadger error reporting

CHANGELOG.rst

@@ -2,6 +2,12 @@
 History
 =======
 
+0.96.0 (2024-07-25)
+-------------------
+* Possible fix for Honeybadger exception masking the actual exceptions.
+* Honeybadger 403 errors will not be raised anymore.
+* Improved FastAPI honeybadger integration.
+
 0.95.1 (2024-07-02)
 -------------------
 * Added socialauth.goog.fetch_info_using_jwt for fetching user info using Google JWT

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/__init__.py

@@ -4,4 +4,4 @@
 
 __author__ = """Scroll Tech"""
 __email__ = "[email protected]"
-__version__ = "0.95.1"
+__version__ = "0.96.0"

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,21 @@ 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.
+            pass
+        else:
+            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,58 @@ 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):
+                err_to_report = None
                 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:
+                        err_to_report = e
+                    raise e
+                except Exception as e:
+                    err_to_report = e
+                    raise e
                 finally:
+                    if err_to_report:
+                        notify_honeybadger(
+                            honeybadger=hb,
+                            error=err_to_report,
+                            func=f,
+                            args=args,
+                            kwargs=kw,
+                        )
+
+            return async_f_wrapped
+
+        else:
+
+            @wraps(f)
+            def f_wrapped(*args, **kw):
+                err_to_report = None
+                try:
+                    return f(*args, **kw)
+                except BaseError as e:
+                    if e.report:
+                        err_to_report = e
+                    raise e
+                except Exception as e:
+                    err_to_report = e
                     raise e
-            return ret
 
-        return f_wrapped
+                finally:
+                    if err_to_report:
+                        notify_honeybadger(
+                            honeybadger=hb,
+                            error=err_to_report,
+                            func=f,
+                            args=args,
+                            kwargs=kw,
+                        )
+
+            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
@@ -59,20 +45,25 @@ def honeybadger_wrapper(hb):
     def wrapper(f):
         @wraps(f)
         def f_wrapped(*args, **kw):
+            err_to_report = None
             try:
                 return f(*args, **kw)
             except BaseError as e:
                 if e.report:
-                    notify_honeybadger(
-                        honeybadger=hb, error=e, func=f, args=args, kwargs=kw
-                    )
+                    err_to_report = e
                 raise e
-
             except Exception as e:
-                notify_honeybadger(
-                    honeybadger=hb, error=e, func=f, args=args, kwargs=kw
-                )
+                err_to_report = e
                 raise e
+            finally:
+                if err_to_report:
+                    notify_honeybadger(
+                        honeybadger=hb,
+                        error=err_to_report,
+                        func=f,
+                        args=args,
+                        kwargs=kw,
+                    )
 
         return f_wrapped
 

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,15 @@
+import asyncio
+from unittest import mock
+
+import pytest
 import requests
 from converge import settings
+from requests.exceptions import HTTPError
 
 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 +319,68 @@ 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
+
+    mocked_honeybadger.notify.side_effect = HTTPError(
+        response=mock.MagicMock(status_code=403)
+    )
+    with pytest.raises(RuntimeError):
+        asyncio.run(wrapped_worst_endpoint(1))
+    # TODO: How to check nested exception?
+    assert mocked_honeybadger.notify.call_count == 3
+
+    mocked_honeybadger.notify.side_effect = HTTPError(
+        response=mock.MagicMock(status_code=401)
+    )
+    with pytest.raises(HTTPError):
+        asyncio.run(wrapped_worst_endpoint(1))
+    # TODO: How to check nested exception?
+    assert mocked_honeybadger.notify.call_count == 4

requirements_dev.txt

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

setup.cfg

@@ -1,5 +1,5 @@
 [bumpversion]
-current_version = 0.95.1
+current_version = 0.96.0
 commit = True
 tag = True
 

setup.py

@@ -42,6 +42,6 @@
     test_suite="tests",
     tests_require=test_requirements,
     url="https://github.com/scrolltech/apphelpers",
-    version="0.95.1",
+    version="0.96.0",
     zip_safe=False,
 )

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