Skip to content

Catch all Historian sink exceptions in EventEmitter.emit (#90) - #98

Merged
randileeharper merged 1 commit into
mainfrom
fix/historian-sink-exceptions-break-operations
Jun 28, 2026
Merged

Catch all Historian sink exceptions in EventEmitter.emit (#90)#98
randileeharper merged 1 commit into
mainfrom
fix/historian-sink-exceptions-break-operations

Conversation

@randileeharper

Copy link
Copy Markdown
Owner

Summary

Fixes #90.

EventEmitter.emit only caught HistorianDeliveryError, but HttpHistorianSink._request re-raises other exceptions — httpx.HTTPStatusError on a 4xx response, ValueError on malformed JSON, httpx.RequestError after exhausting retries — which then propagated out of emit and failed the surrounding user-facing music operation. This violated the design rule that Historian delivery is best-effort.

Change

Broadened emit's except so the best-effort contract is a hard boundary:

  • HistorianDeliveryError → logged at warning (unchanged behavior)
  • any other Exception → caught, logged at error with a traceback (exc_info=True), and never propagated

This keeps genuine sink bugs visible in logs (the visibility concern from issue #47 that originally narrowed the catch to HistorianDeliveryError) while ensuring a Historian outage or bug can never break a pause() / play() / etc. call.

Also hoisted the per-call import logging to a module-level _log logger.

Test changes

Verification

Exact commands run (per AGENTS.md, using the project virtualenv):

.venv/bin/python -m pytest -q                          # 252 passed
.venv/bin/python -m pytest tests/test_historian.py -q  # 18 passed
.venv/bin/python -m ruff check vesper tests            # All checks passed!
.venv/bin/python -m mypy vesper                        # Success: no issues found in 37 source files
.venv/bin/python -m compileall -q vesper tests

Closes #90.

Historian delivery is best-effort: a failed delivery must never fail the
surrounding user-facing music operation. EventEmitter.emit only caught
HistorianDeliveryError, but HttpHistorianSink._request re-raises other
exceptions -- httpx.HTTPStatusError on a 4xx response, ValueError on
malformed JSON, httpx.RequestError after exhausting retries -- which then
propagated out of emit and broke the operation (issue #90).

Broaden emit's except: HistorianDeliveryError is still logged at warning
level; any other exception is now caught, logged at error level with a
traceback (exc_info=True), and never propagated. This makes the best-effort
contract a hard boundary while keeping genuine sink bugs visible in logs
(addressing the visibility concern from issue #47 that originally narrowed
the catch to HistorianDeliveryError).

Also hoists the per-call `import logging` to a module-level `_log` logger.

The issue #47 regression test, which asserted unexpected sink errors
propagate, is updated to assert the new behavior: the operation succeeds and
the unexpected error is logged at error level. A new test covers the
specific httpx.HTTPStatusError (4xx) scenario from issue #90.

Closes #90
@randileeharper
randileeharper merged commit 8bd701b into main Jun 28, 2026
1 check passed
@randileeharper
randileeharper deleted the fix/historian-sink-exceptions-break-operations branch June 28, 2026 16:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Non-delivery exceptions from Historian sink can break operations

1 participant