Skip to content

Fix bag-of-holding log spamming errors on normal operation - #4035

Open
joaoantoniocardoso wants to merge 2 commits into
bluerobotics:masterfrom
joaoantoniocardoso:fix-bag-missing-key
Open

Fix bag-of-holding log spamming errors on normal operation#4035
joaoantoniocardoso wants to merge 2 commits into
bluerobotics:masterfrom
joaoantoniocardoso:fix-bag-missing-key

Conversation

@joaoantoniocardoso

Copy link
Copy Markdown
Member

Example of spam:

ERROR][5:08:58.496 PM -03][service/bag-of-holding][apis.py:32]: 
KeyError

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/home/pi/libs/commonwealth/src/commonwealth/utils/apis.py", line 30, in custom_route_handler
    return await original_route_handler(request)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/blueos/venv/lib/python3.11/site-packages/fastapi/routing.py", line 274, in app
    raw_response = await run_endpoint_function(
                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/blueos/venv/lib/python3.11/site-packages/fastapi/routing.py", line 191, in run_endpoint_function
    return await dependant.call(**values)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/pi/services/bag_of_holding/main.py", line 102, in read_data
    raise HTTPException(status_code=400, detail="Invalid path") from KeyError
fastapi.exceptions.HTTPException

[ERROR][5:09:10.582 PM -03][service/bag-of-holding][apis.py:32]: 
KeyError

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/home/pi/libs/commonwealth/src/commonwealth/utils/apis.py", line 30, in custom_route_handler
    return await original_route_handler(request)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/blueos/venv/lib/python3.11/site-packages/fastapi/routing.py", line 274, in app
    raw_response = await run_endpoint_function(
                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/blueos/venv/lib/python3.11/site-packages/fastapi/routing.py", line 191, in run_endpoint_function
    return await dependant.call(**values)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/pi/services/bag_of_holding/main.py", line 102, in read_data
    raise HTTPException(status_code=400, detail="Invalid path") from KeyError
fastapi.exceptions.HTTPException

@joaoantoniocardoso
joaoantoniocardoso marked this pull request as ready for review July 27, 2026 19:19
@joaoantoniocardoso joaoantoniocardoso added the move-to-stable Needs to be cherry-picked and move to stable label Jul 27, 2026
@github-actions

Copy link
Copy Markdown

Automated PR Review

0. Summary

  • Verdict: LGTM :shipit:

Two small, well-targeted fixes to the log-spam issue reproduced in the PR body:

  1. core/libs/commonwealth/src/commonwealth/utils/apis.py:31-32 — the GenericErrorHandlingRoute now only calls logger.exception() on HTTPExceptions with a 5xx status. Client-side 4xx errors (e.g. the 400 Invalid path raised by bag-of-holding's read_data) no longer produce a full stack trace on every request, which is the correct semantics: 4xx is an expected client outcome, not a service fault.
  2. core/services/bag_of_holding/main.py:101-102raise HTTPException(...) from KeyError was chaining from the KeyError class, not the caught instance, which is why the traceback in the PR body reads KeyError with no message and no origin frame. Switching to from error makes the exception chain point at the actual caught instance, as intended (and matches the pattern used in apis.py:38).

No further comments, nice job 👍

Generated by PR Review Bot. This is advisory, a human reviewer must still approve.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

move-to-stable Needs to be cherry-picked and move to stable

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant