WireGuard's interfaceName method now returns error info #2187
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR updates the WireGuard interfaceName property to use throwing error handling instead of returning optional values, providing more detailed error information when interface name retrieval fails.
- Changed
interfaceNamefrom returningString?to throwingStringwith specific error types - Added two new error cases for buffer allocation failure and system call failure
- Updated all call sites to handle the throwing interface appropriately
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| WireGuardAdapter.swift | Added new error cases and converted interfaceName to throwing property with detailed error handling |
| PacketTunnelProvider.swift | Updated call sites to use try? for optional handling and direct try where errors can propagate |
| WireGuardAdapterError+NetworkProtectionErrorConvertible.swift | Added conversion cases for the new error types |
| NetworkProtectionError.swift | Added corresponding network protection error cases and error code mappings |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| guard let interfaceName = adapter.interfaceName else { | ||
| throw ConnectionTesterError.couldNotRetrieveInterfaceNameFromAdapter | ||
| } | ||
| let interfaceName = try adapter.interfaceName |
There was a problem hiding this comment.
This try statement can throw but the containing function startConnectionTester is not marked as throws. The error should be caught and handled appropriately or the function signature should be updated to propagate the error.
Copilot uses AI. Check for mistakes.
| switch error { | ||
| case NetworkProtectionConnectionTester.TesterError.couldNotFindInterface: | ||
| Logger.networkProtectionConnectionTester.log("Printing current proposed utun: \(String(reflecting: self.adapter.interfaceName), privacy: .public)") | ||
| Logger.networkProtectionConnectionTester.log("Printing current proposed utun: \(String(reflecting: interfaceName), privacy: .public)") |
There was a problem hiding this comment.
The variable interfaceName is now declared inside a do block (line 1638) but is being used outside that scope in the catch block. This will cause a compilation error as interfaceName is not accessible here.
Copilot uses AI. Check for mistakes.
| fire(NetworkProtectionPixelEvent.networkProtectionWireguardErrorInterfaceNameBufferAllocationFailed, | ||
| frequency: .legacyDailyAndCount, | ||
| and: .expect(pixelName: "m_mac_netp_wireguard_error_interface_name_buffer_allocation_failed"), | ||
| file: #filePath, | ||
| line: #line) | ||
| fire(NetworkProtectionPixelEvent.networkProtectionWireguardErrorGetInterfaceNameFailed(TestError.testError), | ||
| frequency: .legacyDailyAndCount, | ||
| and: .expect(pixelName: "m_mac_netp_wireguard_error_get_interface_name_failed", | ||
| error: TestError.testError, | ||
| underlyingErrors: [TestError.underlyingError]), | ||
| file: #filePath, | ||
| line: #line) |
There was a problem hiding this comment.
Need to look into this again as I didn't intend to introduce new pixels here.
1 participant
Task/Issue URL: https://app.asana.com/1/137249556945/project/1203108348835387/task/1211619285056040?focus=true
Description
WireGuard's interfaceName method now returns error info, so we can get more detail on why it can fail.
Testing Steps
interfaceNameImpact and Risks
Low.
What could go wrong?
If done properly this change is low risk - just please make sure none of the changes I did can cause an unexpected regression.
Quality Considerations
NA
Notes to Reviewer
NA
Internal references:
Definition of Done | Engineering Expectations | Tech Design Template
Note
Make WireGuard adapter interfaceName throw rich errors and propagate them through NetworkProtectionError, pixel events, logging, and tests; also bump a dependency.
WireGuardAdapter.interfaceNamenow throws (non-optional), returning POSIX-backed errors; addsinterfaceNameBufferAllocationFailedandgetInterfaceNameFailed.interfaceNameand improved failure logs.NetworkProtectionErrorwithwireGuardInterfaceNameBufferAllocationFailedandwireGuardGetInterfaceNameFailed; map fromWireGuardAdapterErrorand assign error codes/userInfo.MacPacketTunnelProviderand app event mapping; include parameters for underlying errors.duckduckgo-autofillto18.4.0.Written by Cursor Bugbot for commit b45ff1b. This will update automatically on new commits. Configure here.