Skip to content

refactor(gui-client): move change listeners to tunnel service - #8160

Merged
thomaseizinger merged 10 commits into
mainfrom
refactor/move-network-change-listen-ipc-service
Jun 11, 2025
Merged

refactor(gui-client): move change listeners to tunnel service#8160
thomaseizinger merged 10 commits into
mainfrom
refactor/move-network-change-listen-ipc-service

Conversation

@thomaseizinger

@thomaseizinger thomaseizinger commented Feb 17, 2025

Copy link
Copy Markdown
Member

At present, listening for DNS server change and network change events is handled in the GUI client. Upon an event, a message is sent to the tunnel service which then applies the new state to connlib.

We can avoid some of this boilerplate by moving these listeners to the tunnel service as part of the handler. As a result, we get a few improvements:

  • We don't need to ignore these events if we don't have a session because the lifetime of these listeners is tied to the IPC handler on the service side.
  • We need fewer IPC messages
  • We can retry the connection directly from within the tunnel service in case we have no Internet at the time of startup
  • We can more easily model out the state machine of a connlib session in the tunnel service
  • On Linux, this means we no longer shell out to resolvectl from the GUI process, unifying access to the "resolvers" from the tunnel service
  • On Windows, we no longer need admin privileges on the GUI client for optimized network-change detection. This now happens in the Tunnel process which already runs as admin.

Resolves: #9465

@vercel

vercel Bot commented Feb 17, 2025

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
firezone ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 9, 2025 6:18pm

@thomaseizinger

Copy link
Copy Markdown
Member Author

Will need to test this thoroughly. Could run into all kinds of permission / platform issues.

@thomaseizinger
thomaseizinger force-pushed the refactor/move-network-change-listen-ipc-service branch from 6a0d9cc to 1ffedc3 Compare March 17, 2025 10:29
@thomaseizinger

Copy link
Copy Markdown
Member Author

Initial testing on Windows looks good.

@thomaseizinger
thomaseizinger force-pushed the refactor/move-network-change-listen-ipc-service branch 2 times, most recently from 70314c0 to 56914a8 Compare June 7, 2025 14:10
@thomaseizinger
thomaseizinger marked this pull request as ready for review June 7, 2025 14:12
Copilot AI review requested due to automatic review settings June 7, 2025 14:12

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 refactors the GUI client by moving change listeners for DNS and network events into the IPC service, reducing IPC overhead and centralizing session state management. Key changes include replacing the old Option‑based Session handling with a new Session enum, centralizing network change handling in the IPC service, and removing now‑unused event types and fields from multiple modules.

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.

Show a summary per file
File Description
rust/gui-client/src-tauri/src/service.rs Refactors session handling, migrates DNS/network listeners to the handler, and updates connection logic accordingly.
rust/gui-client/src-tauri/src/gui/system_tray/builder.rs Removes obsolete retry event and simplifies the tray menu setup.
rust/gui-client/src-tauri/src/gui/system_tray.rs Updates connection state mappings in the tray menu, removing references to the retired connection retry state.
rust/gui-client/src-tauri/src/controller.rs Removes DNS/network notifier handling and deprecated connection retry logic from the controller.
rust/bin-shared/src/lib.rs Adjusts re-exports to align with the refactored DNS control API.
rust/bin-shared/src/dns_control.rs Removes the public helper for GUI DNS resolution in line with internalizing DNS handling.
Comments suppressed due to low confidence (2)

rust/gui-client/src-tauri/src/service.rs:181

  • [nitpick] The None variant in the Session enum may be ambiguous. Consider renaming it to something more descriptive such as NotConnected to improve code clarity.
    #[default]
    None,

rust/gui-client/src-tauri/src/service.rs:569

  • [nitpick] Rather than using an assert! to ensure the session is in the expected state, consider returning an error to handle the situation gracefully in production.
assert!(self.session.is_none());

@thomaseizinger

Copy link
Copy Markdown
Member Author

@thomaseizinger thomaseizinger changed the title refactor(gui-client): move change listeners to IPC service refactor(gui-client): move change listeners to tunnel service Jun 7, 2025
@thomaseizinger

Copy link
Copy Markdown
Member Author

Needs rebase onto #9475 before it can be tested.

@thomaseizinger

Copy link
Copy Markdown
Member Author

New clients for testing: https://github.com/firezone/firezone/actions/runs/15511322300

@thomaseizinger
thomaseizinger force-pushed the refactor/move-network-change-listen-ipc-service branch from 1d918f7 to 5bcab14 Compare June 9, 2025 11:52
@thomaseizinger

Copy link
Copy Markdown
Member Author

@thomaseizinger

Copy link
Copy Markdown
Member Author

Tested and works on Windows and Ubuntu.

@thomaseizinger
thomaseizinger enabled auto-merge June 9, 2025 21:14

@jamilbk jamilbk left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Think there might be a bug if the administrator has set upstream resolvers

Comment thread rust/gui-client/src-tauri/src/service.rs
@thomaseizinger
thomaseizinger added this pull request to the merge queue Jun 11, 2025
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Jun 11, 2025
@thomaseizinger
thomaseizinger added this pull request to the merge queue Jun 11, 2025
Merged via the queue into main with commit 60bdbb3 Jun 11, 2025
@thomaseizinger
thomaseizinger deleted the refactor/move-network-change-listen-ipc-service branch June 11, 2025 06:33
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.

Moving roaming detection to tunnel service

3 participants