Skip to content

Deprecate synchronous activeURL() in favor of async variant that refreshes network information#4942

Merged
bgoncal merged 2 commits into
mainfrom
claude/async-activeurl-migration-jiukhl
Jul 5, 2026
Merged

Deprecate synchronous activeURL() in favor of async variant that refreshes network information#4942
bgoncal merged 2 commits into
mainfrom
claude/async-activeurl-migration-jiukhl

Conversation

@bgoncal

@bgoncal bgoncal commented Jul 5, 2026

Copy link
Copy Markdown
Member

Summary

The synchronous ConnectionInfo.activeURL() evaluates against network information (current SSID / hardware address) that was cached at some earlier point, so right after a network change it can resolve the wrong URL — e.g. still returning the internal URL after leaving the home network. Several call sites work around this by manually calling Current.connectivity.syncNetworkInformation() first.

This PR deprecates the synchronous API and introduces an async activeURL() that refreshes network information before evaluating which URL is active:

  • ConnectionInfo.activeURL() async — awaits a network-information refresh, then evaluates the active URL. The synchronous activeURL() is now marked @available(*, deprecated, ...).
  • Server.activeURL() async — convenience that refreshes and writes the re-evaluated activeURLType back through Server.info (persisting it), matching what the synchronous mutation did for callers going through server.info.connection.
  • Current.connectivity.refreshNetworkInformation — new World-pattern closure (defaults to syncNetworkInformation()) so tests can control the refresh step.
  • The URL evaluation itself moved to the internal evaluateActiveURL(), used by machinery that must stay synchronous and knowingly accepts cached network information: activeAPIURL(), webhookURL(), ServerRequestAdapter, the ServerManager save path, and ServerInfo.fake().
  • CameraStreamHLSView.fetchStreamURL was migrated to await api.server.activeURL() — it is the one call site inside an async context, where the new overload takes precedence in resolution.
  • New tests in ConnectionInfoTests cover both directions (refresh discovers the internal network / discovers it was left) and the Server convenience.

Remaining synchronous call sites keep compiling and now emit deprecation warnings; migrating them to the async variant is intended as follow-up work.

Screenshots

Not applicable — no user-facing/UI change.

Link to pull request in Documentation repository

Documentation: home-assistant/companion.home-assistant#

Any other notes

  • No behavior change for existing callers: the deprecated function performs exactly the same evaluation as before.
  • Follow-ups: migrate the remaining deprecated call sites, and consider async variants of activeAPIURL()/webhookURL() (the latter also reads isOnInternalNetwork from cached data).

🤖 Generated with Claude Code

https://claude.ai/code/session_01StXKmmfEiUx7S8p1stT4bd


Generated by Claude Code

The synchronous ConnectionInfo.activeURL() evaluates against network
information (current SSID/hardware address) cached at some earlier
point, so it can pick the wrong URL right after a network change —
e.g. returning the internal URL after leaving the home network.

- Add async ConnectionInfo.activeURL(), which refreshes network
  information via the new Current.connectivity.refreshNetworkInformation
  closure before evaluating which URL is active
- Add async Server.activeURL() convenience that does the same and
  persists the re-evaluated active URL type back to the server info
- Deprecate the synchronous ConnectionInfo.activeURL(); the URL
  evaluation now lives in the internal evaluateActiveURL(), used by
  callers that must stay synchronous (activeAPIURL(), webhookURL(),
  ServerRequestAdapter, ServerManager save path, fakes)
- Migrate CameraStreamHLSView.fetchStreamURL to the async variant,
  since the async overload now takes precedence in async contexts
- Cover the new behavior in ConnectionInfoTests

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01StXKmmfEiUx7S8p1stT4bd
Copilot AI review requested due to automatic review settings July 5, 2026 15:23

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR deprecates the synchronous ConnectionInfo.activeURL() and introduces async activeURL() variants that refresh cached network information (SSID/BSSID/hardware address) before choosing the active URL, reducing incorrect URL selection immediately after network changes.

Changes:

  • Added ConnectionInfo.activeURL() async (and deprecated the sync overload), factoring URL selection into evaluateActiveURL() for synchronous call sites that accept cached network state.
  • Added Server.activeURL() async convenience that refreshes network info and persists the re-evaluated activeURLType back to Server.info.
  • Introduced Current.connectivity.refreshNetworkInformation (async) for testability, and migrated CameraStreamHLSView to the async server API; added unit tests covering refresh behavior.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
Tests/Shared/ConnectionInfo.test.swift Adds async unit tests verifying that activeURL() refreshes network info and that Server.activeURL() updates persisted activeURLType.
Sources/Shared/Environment/ConnectivityWrapper.swift Adds async refreshNetworkInformation hook on the connectivity wrapper for refreshing cached network details (test-overridable).
Sources/Shared/API/ServerManager.swift Switches synchronous active URL evaluation to the new evaluateActiveURL() helper.
Sources/Shared/API/Server+Fakes.swift Updates fake server info initialization to use evaluateActiveURL().
Sources/Shared/API/Server.swift Adds Server.activeURL() async convenience that refreshes network information and persists the evaluated URL type.
Sources/Shared/API/ConnectionInfo.swift Deprecates sync activeURL(), adds async activeURL(), introduces evaluateActiveURL() for synchronous internal consumers, and updates related call sites.
Sources/App/Cameras/CameraPlayer/CameraStreamHLSView.swift Migrates HLS base URL lookup to await api.server.activeURL() inside async context.

Comment thread Tests/Shared/ConnectionInfo.test.swift
Comment thread Tests/Shared/ConnectionInfo.test.swift
Comment thread Tests/Shared/ConnectionInfo.test.swift
Comment thread Sources/Shared/Environment/ConnectivityWrapper.swift
…ure in tests

- ConnectivityWrapper.refreshNetworkInformation now defaults to calling
  syncNetworkInformation() on the instance (lazy, weak self) instead of
  going through the global Current environment
- Tests capture the previous refreshNetworkInformation closure and
  restore it in teardown rather than resetting to a hard-coded default

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01StXKmmfEiUx7S8p1stT4bd
@codecov

codecov Bot commented Jul 5, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (main@d55d436). Learn more about missing BASE report.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4942   +/-   ##
=======================================
  Coverage        ?   48.75%           
=======================================
  Files           ?      283           
  Lines           ?    18018           
  Branches        ?        0           
=======================================
  Hits            ?     8785           
  Misses          ?     9233           
  Partials        ?        0           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@bgoncal bgoncal merged commit dba0eb7 into main Jul 5, 2026
15 checks passed
@bgoncal bgoncal deleted the claude/async-activeurl-migration-jiukhl branch July 5, 2026 21:06
bgoncal added a commit that referenced this pull request Jul 9, 2026
…t connection on launch (#5017)

## Summary
Since URL resolution moved to `async` (#4942 / #4953), every path that
picks the active URL awaits a fresh `NEHotspotNetwork.fetchCurrent`.
That completion handler occasionally never fires (seen at
launch/foreground, before location services are ready), and there was no
timeout, so `refreshNetworkInformation()` / `activeURL()` would hang
indefinitely. Everything awaiting them then stalls at once:

- the web view load hangs with `loadActiveURLIfNeededInProgress` stuck
`true`, so no later event retries it — blank page that never reloads
- the WebSocket never connects (the cold-connect `Task` never reaches
`connectAPI`)
- the account row shows no user (`currentUser` never completes)

This matches the intermittent reports of a blank web view that won't
reload, servers listed with no username, and no WebSocket connection.

Fix: bound `performNetworkStateFetch()` with a 3s timeout. If the fetch
doesn't return in time, keep the last-known network state instead of
hanging (or overwriting it with an empty one). Callers always make
progress, and the normal launch/foreground events re-drive the
connection once network info is available. Catalyst reads network info
synchronously and is unaffected.

## Screenshots
Not applicable — no user-facing/UI change.

## Link to pull request in Documentation repository
Documentation: home-assistant/companion.home-assistant#

## Any other notes
- 3s is a safety cap only; `fetchCurrent` normally returns well under a
second.
- On timeout the previous value is preserved rather than cleared, so a
slow fetch can't flip the app to the wrong URL.

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
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.

3 participants