Skip to content

[backport/1.4] Keep Ping port watcher running after unexpected errors - #4052

Draft
joaoantoniocardoso wants to merge 1 commit into
bluerobotics:1.4-devfrom
joaoantoniocardoso:backports/1.4/ping-port-watcher-resilience
Draft

[backport/1.4] Keep Ping port watcher running after unexpected errors#4052
joaoantoniocardoso wants to merge 1 commit into
bluerobotics:1.4-devfrom
joaoantoniocardoso:backports/1.4/ping-port-watcher-resilience

Conversation

@joaoantoniocardoso

@joaoantoniocardoso joaoantoniocardoso commented Jul 27, 2026

Copy link
Copy Markdown
Member

This is a backport of #4040 into 1.4.


Supersedes #4042 (recreated from fork joaoantoniocardoso/BlueOS-docker instead of same-repo head).

@github-actions

Copy link
Copy Markdown

Automated PR Review

0. Summary

  • Verdict: MINOR SUGGESTIONS ✏️

Backport of #4040 to 1.4-dev. Wraps the body of PortWatcher.start_watching's main loop in a try/except Exception so that transient failures (e.g. find_ping360_ethernet raising KeyError on a disappearing interface) no longer kill the watcher task. The await asyncio.sleep(1) is intentionally left outside the try, which correctly ensures the loop still yields between iterations on failure. Adds a small pytest checking that the loop keeps running after add_ping360 raises.

6. Code Quality & Style

  • 6.1 [nit] core/services/ping/portwatcher.py:42logger.exception(f"Error while watching ports/devices: {error}") interpolates the exception into the message, but logger.exception already appends the traceback (including the stringified exception) via sys.exc_info(). The {error} interpolation is redundant; a plain logger.exception("Error while watching ports/devices") is enough.

7. Tests

  • 7.1 [nit] core/services/ping/test_portwatcher.py:19 — using asyncio.CancelledError to break out of start_watching is clever but subtle: because CancelledError inherits from BaseException (not Exception) in Python 3.11, it escapes the new except Exception block by design. That is the correct behavior, but a one-line comment on why CancelledError is used as the loop terminator would save the next reader a trip to the docs. Optional.

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

@joaoantoniocardoso
joaoantoniocardoso marked this pull request as draft July 27, 2026 21:34
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.

1 participant