Fiddle: tidy up exception handling in api and message processor#128
Conversation
There was a problem hiding this comment.
Pull request overview
This PR refines exception handling for both background message processing and the HTTP API middleware to provide more structured error information and configurable logging of client errors.
Changes:
- In
MessageQueueProcessor._process_message, wrap arbitrary exceptions withKibaException.from_exceptionand send only the normalizedkibaException.messagein notification payloads. - In
ExceptionHandlingMiddleware, add a configurableshouldSquashClientExceptionsflag and corresponding__init__, allowing toggling between squashed error logs and full stack traces forClientExceptions.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
core/queues/message_queue_processor.py |
Normalizes caught exceptions to KibaException before notifying clients and switches notification content to use the structured exception message. |
core/api/middleware/exception_handling_middleware.py |
Adds an explicit constructor with a shouldSquashClientExceptions toggle and updates ClientException handling to optionally log full stack traces instead of squashed error messages. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if self.shouldSquashClientExceptions: | ||
| logging.error(f'{exception.exceptionType} occurred: {exception.message}') | ||
| else: | ||
| logging.exception(exception) |
There was a problem hiding this comment.
The new shouldSquashClientExceptions flag introduces an alternate logging path for ClientException (the else: logging.exception(exception) branch), but there are currently no tests exercising this behavior (e.g. constructing the app with ExceptionHandlingMiddleware(shouldSquashClientExceptions=False) and asserting stack traces are logged / responses unchanged). Consider adding a test case alongside the existing middleware tests (e.g. in tests/api/test_json_route.py or test_streaming_json_route.py) to cover the non-squashed path and guard against regressions in this error-handling behavior.
Description
Screenshots:
Checklist: