fix(netwatch): discover Windows default routes without WMI - #226
domenkozar wants to merge 4 commits into
Conversation
matheus23
left a comment
There was a problem hiding this comment.
Thank you for this PR!
I checked
- The error enum changes are private, so don't break the API
- The original
default_interfaceimplementation was introduced here, there's no particular reason given why it's using WMI's RouteTable lookup vs. netdev (default-net back then).
We use default routes in iroh for checking if the network seemingly changed (e.g. Wifi to Cellular).
Do you happen to have a windows machine to test this behavior? When switching networks, you should see netwatch::netmon::Monitor::interface_change() trigger a change and have State::is_major_change relative to the past version be true.
Otherwise this LGTM.
|
I sadly don't have access to a windows machine, I've observed this on Windows CI and fixed it along the way. |
|
Hm okay. I asked dig why he introduced WMI-based route resolution specifically for windows in that old iroh PR, and he says it was because netdev wasn't as reliable. |
(need to run further tests on Windows)
Not sure it counts, but I vibecoded a test in Windows CI at https://github.com/domenkozar/net-tools/actions/runs/34504089122/job/102961810876 |
|
@matheus23 any chance to merge this? |
|
Yeah - this is just blocked on proper testing on a windows box with network changes. Sorry - the team was busy attending RustConf and a team retreat (and I was on vacation last week). I'll have access to a windows box next week. |
|
I'm writing to encourage merging this PR. The rest of this note was generated by Claude but directly addresses a real issue we hit that forced us to remove a feature from our app on Windows builds... Another benefit of this PR: the WMI dependency also makes
[target.'cfg(target_os = "windows")'.dependencies]
windows = ">=0.59, <0.63"
windows-core = ">=0.59, <0.63"In a graph that contains both note: there are multiple different versions of crate That fails the entire Windows build, not just the networking code. Replacing the WMI path as this PR does removes the failure mode completely, so it would fix tauri + iroh on Windows as a side effect. Provenance, to be clear about what we did and didn't verify: this comes from CI build logs for x86_64-pc-windows-msvc plus the resolved |
|
@JamesLavin thanks for letting us know. Do you currently have a windows machine accessible to you and can run a quick test? (I'm currently traveling this week, so don't have access to my usual machines) Either run a specific test with netwatch main and netwatch under this PR to see how it handles network changes (e.g. switching between WiFi networks or switching from WiFi to Ethernet), or run iroh with and without this PR patched in and see if the transfer example in the iroh repository works across network changes in both versions. |
|
Thanks for replying, @matheus23. I do my dev work on Mac & Linux but have a Windows laptop I can test on. I pulled the PR branch, installed the latest Rustup, and ran All tests passed, except for I can run whichever tests/commands you want, but I'm not familiar with the repo, and I put my |
Description
Windows default-route discovery currently creates a WMI connection, which requires COM access. In Factorseal's restricted Windows network helper, that path terminated the process with delay-load exception
0xc06d007eduring network startup.Use the existing
netdev::get_default_interface()lookup instead. This avoids WMI and returns an interface name from the same source asget_state(), matching the documented requirement thatState::default_route_interfacebe a key inState::interfaces. Synchronous discovery stays onspawn_blocking, and lookup failures still produce a warning andNone.Remove the WMI dependency and its now-unused Windows-only
chronoandserdedeclarations. Add a Windows test checking the route-name/map-key contract when a default route is available.Breaking Changes
None. No public API changes.
Notes & open questions
Validation:
cargo test --locked -p netwatch --all-features --lib --test smoke: 17 library tests and one smoke test pass on Linux.cargo xwin clippy --locked --target x86_64-pc-windows-msvc -p netwatch --all-features --all-targets -- -D warningspasses, including compilation of the new Windows test.The new upstream Windows test has been cross-compiled locally; its native execution is left to upstream CI. Factorseal's native results exercise the downstream integration, not that new test.
Change checklist