Skip to content

fix(osc): wait and log once when an OSC client cannot be created - #492

Merged
michelhonold merged 3 commits into
mainfrom
fix/osc-unresolvable-destination-main
Oct 24, 2026
Merged

michelhonold merged 3 commits into
mainfrom
fix/osc-unresolvable-destination-main

Conversation

@michelhonold

@michelhonold michelhonold commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

Plan

  • Regression test that fails on main: 600 sends to an unresolvable destination log 600 ERROR lines and look the name up 600 times
  • Wait 1 s before retrying any destination whose client cannot be created, pinned or not
  • Log a creation failure per streak: one WARNING, a reminder at most once a minute, one INFO on recovery
  • Network guide
  • Three review rounds

Summary

Part of #369.

An OSC destination whose client cannot be created, most often a host name the show network's resolver cannot answer, was retried on every send and logged an ERROR each time. A transmitter row streaming at its frame rate wrote thousands of lines a minute and buried the log tail the diagnostics bundle carries. Only a destination with an interface pin waited before retrying.

Now every destination whose client cannot be created (a name that does not resolve, a refused interface pin, a malformed host) waits 1 s before the next attempt. Its failures are logged per streak:

  • one WARNING when it starts: Cannot create OSC client for udp://<host>:<port>, retrying every 1s: <reason>
  • a reminder with the attempt count at most once a minute while it keeps failing
  • one INFO when the client is finally created, with the number of failed attempts

An interface eviction still retries a pinned destination at once, but keeps its streak, so an address change does not announce the failure again.

This is the main-only part of #369. The issue also asks for a row naming the destination on the Operator Screen and in Statistics. That row needs the per-destination send records from #359, which targets another branch, so it is not in this PR and #369 stays open for it.

What #359 replaces or extends

When #359 lands, the per-streak logging in OscService._record_create_failure (and _CreateFailure / _create_failures) gives way to recording the failed creation in that destination's SendHealth, which then owns the log lines and puts the row on screen. The 1 s retry for every destination (_CREATE_RETRY_S, renamed from _PINNED_CREATE_RETRY_S) stays as it is. Expect one small conflict at the new_entry = _ClientEntry(...) line in _get_or_create_client, which #359 also changes.

Behaviour notes

  • The log level for a failed creation drops from ERROR to WARNING. Nothing parses the old Failed to create OSC client line.
  • A one-shot send (a trigger zone event) to a destination that failed less than 1 s ago is dropped without a new attempt. Before, it was attempted and would almost always have failed the same way.

Test plan

  • tests/test_osc_service.py::TestClientCreationFailure: 600 sends make one lookup and one WARNING; a 150 s streak logs the start, reminders after 61 and 121 attempts, and one INFO on recovery; a new streak after a recovery starts afresh; an eviction retries at once and keeps the streak; an unresolvable name, a refused pin and a malformed host all wait 1 s (parametrized)
  • The regression tests fail on main (6 of 7 failed before the fix; the refused-pin case already waited)
  • Mutation check: dropping the unpinned retry, the reminder throttle, the reminder timestamp, the attempt count, the recovery log, or clearing the streak on recovery or eviction each fails a test
  • 100 % coverage of the changed lines in openfollow/osc/service.py
  • mypy, ruff check, ruff format
  • make ci-remote
  • Station check: an OSC destination with a host name that does not resolve logs one WARNING, then one reminder a minute, and logs one INFO and starts sending once the name resolves
    • The name was made to resolve with a temporary hosts entry; packets arrived at the destination right after the INFO line.

🤖 Generated with Claude Code

michelhonold and others added 3 commits October 21, 2026 05:38
A destination whose client cannot be created, most often a host name the
show network's resolver cannot answer, was retried on every send and
logged an ERROR each time: a row streaming at its frame rate wrote
thousands of lines a minute and buried the log tail the diagnostics
bundle carries. Only a pinned target waited before retrying.

Every target whose client cannot be created (unresolvable name, refused
pin, malformed host) now waits 1 s before the next attempt, and its
failures are logged per streak: one WARNING when it starts, a reminder
with the attempt count at most once a minute, one INFO when the client
is finally created. An eviction still clears the retry delay but keeps
the streak, so an address change does not announce it again.

Refs #369

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@michelhonold michelhonold added the bug Something isn't working label Oct 21, 2026
@michelhonold

Copy link
Copy Markdown
Contributor Author

Review rounds

Each round reviewed the whole git diff origin/main HEAD as a fresh reviewer.

Round 1

  • Medium – the network guide described eviction-keeps-the-streak, but no test pinned it: a change that cleared the streak on eviction passed. Added test_an_eviction_retries_at_once_but_keeps_the_streak, mutation-checked (clearing _create_failures in _drop_pinned fails it). b94f2d2
  • Low – the first test's name said "logged once per retry"; it is logged once per streak. Renamed to ..._is_tried_once_per_retry_and_logged_once. 28207bd
  • Low – the guide said "an eviction clears the retry delay"; only an interface eviction (evict_egress / retain_egress) does, and only for a pinned target. Reworded. 28207bd
  • Cosmetic – _send_for's docstring called the target unresolvable while the recovery test also uses it after it resolves. Reworded. 28207bd

Round 2

  • Checked thread safety: the retry time, streak and attempt count change under _cache_lock; the log calls run outside it. Two threads failing at once count two attempts and log one start line. Two threads creating a client at once log one INFO (the first to pop the streak). No finding.
  • Checked the clock in the tests: every step is a binary-exact quarter second from a fixed base, so the 61 / 121 / 150 counts do not depend on the host. No finding.
  • Checked merge shape against fix(network): report refused sends and name macOS Local Network access #359: the retry constant, its comment and the _create_retry_at comment match the fuller fix line for line; the only overlap is the new_entry = _ClientEntry(...) line. Noted in the PR description. No finding.

Round 3

  • Nothing beyond rare cases.

Left as is

  • Rare – a destination that keeps failing and is then removed from the config keeps a small streak record for the life of the process. The records are bounded by the number of distinct destinations ever used, and fix(network): report refused sends and name macOS Local Network access #359 replaces this state.
  • Rare – a trigger zone event sent within 1 s of a failed creation is dropped without a new attempt. It would almost always have failed the same way; noted in the PR description.

Results

  • tests/test_osc_service.py, tests/test_osc_transmitter.py, tests/test_services_startup.py, tests/test_services_zone_orchestration.py: 573 passed
  • 7 mutations of the fix plus 1 of eviction, all killed
  • Changed lines in openfollow/osc/service.py fully covered
  • mypy, ruff check and ruff format clean

@michelhonold
michelhonold marked this pull request as ready for review October 24, 2026 19:24
@michelhonold
michelhonold merged commit e36de3b into main Oct 24, 2026
4 checks passed
@michelhonold
michelhonold deleted the fix/osc-unresolvable-destination-main branch October 24, 2026 19:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant