Bug/network type switch crash swift logs #39
Conversation
Message: When switching between wifi and cellular, the Go engine fires rapid state changes (connecting → disconnecting → connected) that confused the UI animation state machine, causing it to get stuck in "Connecting..." state. Changes: - Add isRestarting flag to suppress intermediate state updates during restart - Add recovery path in playDisconnectingLoop when engine reconnects - Add status polling guard to prevent concurrent fetchData calls - Add AppLogger for unified Swift logging to shared container - Update share logs to export both engine and app log files
WalkthroughAdds a unified AppLogger and integrates it across app and extension; changes adapter restart flow with an isRestarting flag and stop(completionHandler:); prevents concurrent status fetches; aggregates log sharing; tweaks a Lottie disconnecting loop; makes Firebase initialization optional; and updates Xcode project references, build settings, and CI workflows. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant PT as PacketTunnelProvider
participant AD as NetBirdAdapter
participant CL as ConnectionListener
participant LOG as AppLogger
PT->>LOG: log "restart initiated"
PT->>AD: set isRestarting = true
PT->>AD: stop(completionHandler)
activate AD
AD->>CL: onDisconnecting (CL sees isRestarting and suppresses transition)
CL->>LOG: log "transition suppressed (isRestarting=true)"
AD->>AD: perform stop
AD->>AD: on stop complete -> notifyStopCompleted()
AD->>LOG: log "stop completed, starting"
AD->>AD: start adapter
AD->>CL: onConnecting (suppressed if still isRestarting)
CL->>LOG: log "transition suppressed (isRestarting=true)"
AD->>CL: onConnected
CL->>CL: set isRestarting = false
CL->>LOG: log "connected (restart=false)"
deactivate AD
PT->>LOG: log "restart finished"
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
mlsmaycon
changed the base branch from
main
to
bug/network-type-switch-crash
mlsmaycon
changed the base branch from
bug/network-type-switch-crash
to
main
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
NetbirdKit/NetworkExtensionAdapter.swift (1)
256-270: Missing completion call when decoding fails.If
decoder.decodethrows at line 258, the catch block logs the error but doesn't callcompletionbefore the closure ends. This leaves the caller without a response.if let response = response { do { let decodedStatus = try self?.decoder.decode(StatusDetails.self, from: response) if let status = decodedStatus { completion(status) } return } catch { AppLogger.shared.log("Failed to decode status details.") + let defaultStatus = StatusDetails(ip: "", fqdn: "", managementStatus: .disconnected, peerInfo: []) + completion(defaultStatus) + return } } else { let defaultStatus = StatusDetails(ip: "", fqdn: "", managementStatus: .disconnected, peerInfo: []) completion(defaultStatus) return }NetBird/Source/App/Views/AdvancedView.swift (1)
244-268: Remove unusedsaveLogFile(at:)method.The
saveLogFile(at:)method is not called anywhere and appears to be leftover from the previous log-sharing implementation. Removing it will reduce code clutter.Apply this diff:
- func saveLogFile(at url: URL?) { - guard let url = url else { return } - - let fileManager = FileManager.default - guard let groupURL = fileManager.containerURL(forSecurityApplicationGroupIdentifier: "group.io.netbird.app") else { - print("Failed to retrieve the group URL") - return - } - - let logURL = groupURL.appendingPathComponent("logfile.log") - - do { - let logData = try String(contentsOf: logURL, encoding: .utf8) - let fileURL = url.appendingPathComponent("netbird.log") - do { - try logData.write(to: fileURL, atomically: true, encoding: .utf8) - print("Log file saved successfully.") - } catch { - print("Failed to save log file: \(error)") - } - } catch { - print("Failed to read log data: \(error)") - return - } - }
♻️ Duplicate comments (1)
.github/workflows/build.yml (1)
27-31: Verify the Go version - 1.24 may not exist yet.The workflow specifies Go version '1.24', which may not be released yet. This is the same issue as in
.github/workflows/test.yml. Both workflow files should use a valid Go version.
🧹 Nitpick comments (2)
NetBird/Source/App/Views/Components/CustomLottieView.swift (1)
166-176: Good recovery path; consider wrapping UIView update in the same async block for consistency.The logic correctly handles the scenario where the engine recovers to connected during an internal restart (network switch). However,
uiView.currentFrameis set outsideDispatchQueue.main.async, which is inconsistent with howplayFadeOuthandles state updates (whereisPlayingis set inside the async block). While Lottie completion handlers typically run on the main thread, wrapping all state and UI updates together ensures consistency and guards against potential threading issues.} else if self.engineStatus == .connected && self.extensionStatus == .connected { // Engine recovered to connected during internal restart (e.g., network switch) // Extension never disconnected, so skip fade out and go directly to connected state - self.isPlaying = false DispatchQueue.main.async { + self.isPlaying = false viewModel.extensionStateText = "Connected" viewModel.connectPressed = false viewModel.disconnectPressed = false viewModel.routeViewModel.getRoutes() + uiView.currentFrame = self.connectedFrame } - uiView.currentFrame = self.connectedFrame }README.md (1)
13-18: Add alt text to badge images for accessibility.The static analysis tool flagged that badge images on lines 14 and 17 are missing alt text attributes. Consider adding descriptive
altattributes.<a href="https://github.com/netbirdio/ios-client/actions/workflows/build.yml"> - <img src="https://github.com/netbirdio/ios-client/actions/workflows/build.yml/badge.svg"/> + <img src="https://github.com/netbirdio/ios-client/actions/workflows/build.yml/badge.svg" alt="Build Status"/> </a> <a href="https://github.com/netbirdio/ios-client/actions/workflows/test.yml"> - <img src="https://github.com/netbirdio/ios-client/actions/workflows/test.yml/badge.svg"/> + <img src="https://github.com/netbirdio/ios-client/actions/workflows/test.yml/badge.svg" alt="Test Status"/> </a>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
.github/workflows/build.yml(1 hunks).github/workflows/test.yml(1 hunks)NetBird.xcodeproj/project.pbxproj(9 hunks)NetBird/Source/App/NetBirdApp.swift(1 hunks)NetBird/Source/App/Views/AdvancedView.swift(1 hunks)NetBird/Source/App/Views/Components/CustomLottieView.swift(1 hunks)NetbirdKit/AppLogger.swift(1 hunks)NetbirdKit/ConnectionListener.swift(1 hunks)NetbirdKit/NetworkExtensionAdapter.swift(3 hunks)NetbirdNetworkExtension/NetBirdAdapter.swift(3 hunks)NetbirdNetworkExtension/PacketTunnelProvider.swift(3 hunks)README.md(2 hunks)build-go-lib.sh(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
NetBird/Source/App/Views/AdvancedView.swift (1)
NetbirdKit/AppLogger.swift (3)
log(80-90)getGoLogFileURL(139-156)getLogFileURL(131-137)
NetbirdKit/NetworkExtensionAdapter.swift (1)
NetbirdKit/AppLogger.swift (1)
log(80-90)
NetbirdNetworkExtension/NetBirdAdapter.swift (1)
NetbirdKit/NetworkExtensionAdapter.swift (1)
stop(136-138)
NetBird/Source/App/Views/Components/CustomLottieView.swift (2)
NetbirdKit/NetworkExtensionAdapter.swift (1)
getRoutes(166-197)NetBird/Source/App/ViewModels/RoutesViewModel.swift (1)
getRoutes(54-59)
🪛 markdownlint-cli2 (0.18.1)
README.md
14-14: Images should have alternate text (alt text)
(MD045, no-alt-text)
17-17: Images should have alternate text (alt text)
(MD045, no-alt-text)
🔇 Additional comments (21)
README.md (1)
64-95: LGTM!The README updates are well-structured: updated requirements align with CI workflows (Xcode 16+, Go 1.23+), the "Run locally" section is clearer with explicit commands, and the optional Firebase configuration section documents the new safe initialization pattern introduced in the code.
NetBird.xcodeproj/project.pbxproj (2)
94-102: LGTM!The project structure changes correctly integrate
AppLogger.swiftinto both the main app and network extension targets, enabling unified logging across the codebase. The file organization follows existing patterns.Also applies to: 318-318, 585-585, 619-619
846-846: Version bump noted.Version updated from 0.0.13 to 0.0.14 with build number 7, appropriate for this bug fix release.
Also applies to: 875-875, 898-898, 927-927
build-go-lib.sh (1)
19-19: LGTM!The output path change aligns with the updated project structure where
NetBirdSDK.xcframeworkis now located under theNetBird/subdirectory, matching the reference in the Xcode project file.NetBird/Source/App/NetBirdApp.swift (1)
15-18: LGTM - safe Firebase initialization.Good defensive change using optional binding instead of force-unwrap. This prevents crashes when
GoogleService-Info.plistis missing and aligns with the optional Firebase documentation in the README.Consider adding a log message when Firebase configuration is skipped, so developers are aware:
if let path = Bundle.main.path(forResource: "GoogleService-Info", ofType: "plist"), let options = FirebaseOptions(contentsOfFile: path) { FirebaseApp.configure(options: options) + } else { + print("Firebase configuration skipped - GoogleService-Info.plist not found") }NetbirdKit/NetworkExtensionAdapter.swift (1)
240-250: Good addition of status polling guard.The
isFetchingStatusflag correctly prevents concurrentfetchDatacalls, which addresses the rapid state transition issue during network switches. The pattern withdeferfor cleanup andweak selfto avoid retain cycles is appropriate.NetbirdNetworkExtension/PacketTunnelProvider.swift (2)
32-35: LGTM - consistent Firebase configuration.Same safe optional binding pattern as in the main app, ensuring consistent behavior across both the app and network extension.
109-137: LGTM - improved network change handling with logging.Good use of
AppLoggerfor network change diagnostics. The logging will help debug network type switch issues.NetbirdKit/ConnectionListener.swift (4)
25-34: LGTM!The
onConnectedcallback correctly handles the restart flag by capturing the previous state, resettingisRestarting, updating the client state, and logging the transition. The completion handler is properly invoked on the main queue.
36-43: LGTM! State suppression during restart prevents UI confusion.The conditional suppression of the "connecting" state when
isRestartingis true prevents intermediate state transitions from confusing the UI animation state machine, which aligns with the PR's objective.
45-51: LGTM! Proper integration with stop completion flow.The
onDisconnectedcallback correctly resets theisRestartingflag, updates state, and callsadapter.notifyStopCompleted()to trigger any pending stop completion handlers. This ensures the stop lifecycle is properly managed.
53-60: LGTM! Consistent suppression pattern.The suppression of the "disconnecting" state during restart mirrors the
onConnectingbehavior and maintains consistency in preventing intermediate state updates.NetBird/Source/App/Views/AdvancedView.swift (1)
189-237: LGTM! Improved log sharing with dual-source aggregation.The refactored
shareButtonTapped()method correctly:
- Gathers logs from both Go SDK and Swift app via
AppLoggerAPIs- Writes them to the documents directory with descriptive filenames (
netbird-engine.log,netbird-app.log)- Shares all available logs through a single activity controller
- Uses
AppLogger.shared.logfor error reporting instead of- Guards against presenting an empty share sheet
NetbirdKit/AppLogger.swift (5)
10-31: LGTM! Well-structured singleton with async initialization.The
AppLoggerclass properly implements a singleton pattern with asynchronous file setup on a background queue, preventing main thread blocking during initialization. The use ofweak selfin the async block avoids potential retain cycles.
33-78: LGTM! Robust file setup with fallback strategy.The
setupLogFile()method correctly handles:
- App group container with fallback to documents directory (Mac Catalyst support)
- Directory and file creation with proper error handling
- File handle initialization with append positioning
The error handling prints to console, which is appropriate since the logger cannot use itself during initialization.
80-99: LGTM! Effective dual-output logging with async writes.The
log()method correctly:
- Formats messages with timestamp and source location
- Outputs to both console and file
- Performs file writes asynchronously to avoid blocking
Note: Line 98 uses
try?to silently suppresssynchronize()errors, which is acceptable for logging infrastructure where failure to sync is not critical.
101-115: LGTM! Proper log rotation with size threshold.The rotation logic correctly checks file size and performs cleanup when exceeding 5MB. The file handle is properly closed before removal and reopened after recreation. Error handling is appropriate for this context.
117-156: LGTM! Well-designed public APIs for log management.The public methods provide clean interfaces for:
clearLogs(): Properly closes the handle before clearing, ensuring safe file operationsgetLogFileURL(): Returns the Swift log file URL with existence checkgetGoLogFileURL(): Searches both app group and documents directory for Go SDK logs, providing flexibilityNetbirdNetworkExtension/NetBirdAdapter.swift (2)
28-31: LGTM! Clear documentation for restart suppression flag.The
isRestartingflag is well-documented and appropriately scoped asinternalto enable coordinated restart behavior across the module. The documentation clearly explains its purpose in preventing UI confusion during network type switches.
145-149: LGTM! Proper idempotent completion handler invocation.The
notifyStopCompleted()method correctly:
- Guards against nil handler (making it safe to call multiple times)
- Clears the handler before invoking (preventing re-entry)
- Ensures completion handler is invoked exactly once
This idempotent behavior properly handles scenarios where both the timeout and
onDisconnectedfire..github/workflows/test.yml (1)
27-31: Remove concern about Go 1.24 not existing.Go 1.24 has been released and is a valid version as of December 2025. The latest stable version is Go 1.25.5, so if keeping current with the latest releases is important, consider upgrading to 1.25.x.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/build.yml(1 hunks).github/workflows/test.yml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/build.yml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build iOS App
- GitHub Check: Build for Simulator
🔇 Additional comments (2)
.github/workflows/test.yml (2)
1-11: Workflow structure and concurrency control look good.The workflow is well-structured with proper trigger configuration and concurrency limits to prevent duplicate runs on rapid pushes.
36-37: Build script path and netbird integration verified.The
build-go-lib.shscript exists in the repository root and correctly handles the netbird path argument. The script accepts the path as its first parameter (line 5) with a fallback to${GOPATH}/src/github.com/netbirdio/netbird, and uses it consistently throughout the gomobile build process (lines 15 and 19). Passing$PWD/netbirdas the argument is correct and will be properly utilized by the script.
| - name: Setup Go | ||
| uses: actions/setup-go@v5 | ||
| with: | ||
| go-version: '1.24' | ||
| cache-dependency-path: netbird/go.sum | ||
|
|
||
| - name: Install gomobile | ||
| run: go install golang.org/x/mobile/cmd/gomobile@v0.0.0-20251113184115-a159579294ab |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Go 1.24 release date stable version
💡 Result:
Go 1.24 (stable) was released on February 11, 2025. [1][2]
Sources:
[1] Go blog — "Go 1.24 is released!" (Feb 11, 2025).
[2] Go release history — go1.24.0 (released 2025-02-11).
🌐 Web query:
gomobile golang.org/x/mobile latest stable version
💡 Result:
There is no published semver "stable" release for golang.org/x/mobile (gomobile). Install the latest module commit with:
go install golang.org/x/mobile/cmd/gomobile@latest
The module on pkg.go.dev shows no tagged version, and the source is maintained in the golang mobile repo (recent commits). [1][2]
Sources:
[1] pkg.go.dev — golang.org/x/mobile.
[2] go.googlesource.com/mobile (repository commit history).
Use @latest for gomobile instead of pinning to a development commit.
Go 1.24 is stable and available (released February 11, 2025). However, gomobile has no published semver releases. The commit 2025-11-13 is a development commit pinned to a specific point in time, making it brittle for CI. Use @latest instead to get the most recent stable development version of gomobile.
🤖 Prompt for AI Agents
.github/workflows/test.yml lines 27-34: the workflow pins gomobile to a specific
development commit which is brittle; change the gomobile install step to use the
module proxy latest tag by replacing the @<commit> suffix with @latest so the CI
installs the most recent compatible gomobile version instead of a fixed dev
commit.
| - name: Build for Simulator | ||
| run: | | ||
| set -o pipefail | ||
| xcodebuild build \ | ||
| -project NetBird.xcodeproj \ | ||
| -scheme NetBird \ | ||
| -destination 'platform=iOS Simulator,name=iPhone 16' \ | ||
| -configuration Debug \ | ||
| CODE_SIGNING_ALLOWED=NO \ | ||
| | xcpretty --color No newline at end of file |
There was a problem hiding this comment.
Missing test execution and no artifact uploads.
The workflow only builds the app but does not execute any actual tests (the unit test step is a no-op placeholder). Additionally, there are no artifact uploads for build logs or binaries, which is particularly important given this PR adds unified logging (AppLogger). Without artifact uploads, CI logs and test results won't be preserved for debugging or audit.
Consider: (1) adding explicit test execution instead of the placeholder, (2) uploading build logs on failure, and (3) optionally uploading app binaries for integration testing.
Consider adding artifact uploads after the build step:
- name: Upload build logs on failure
if: failure()
uses: actions/upload-artifact@v4
with:
name: build-logs
path: |
build/
*.log
retention-days: 7Also replace the unit test placeholder with an actual test step or remove it if tests are not ready yet.
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (4)
NetBird/Source/App/Views/AdvancedView.swift (1)
189-237: Don’t do synchronous log IO on the main thread
String(contentsOf:)/ writes can hitch the UI. Consider moving export work off-main, then present onMainActor.func shareButtonTapped() { + Task.detached(priority: .utility) { + // do export work here (build filesToShare) + // ... + await MainActor.run { + // present UIActivityViewController here + } + } }NetbirdKit/NetworkExtensionAdapter.swift (2)
27-27: Consider makingisFetchingStatusthread-safe.The
isFetchingStatusflag is set on the main thread (via Timer) but reset insidesendProviderMessagecompletion handler which may execute on a different thread. This could lead to a race condition.Consider using a serial dispatch queue or atomic property wrapper:
- private var isFetchingStatus = false + private let fetchLock = NSLock() + private var _isFetchingStatus = false + private var isFetchingStatus: Bool { + get { fetchLock.lock(); defer { fetchLock.unlock() }; return _isFetchingStatus } + set { fetchLock.lock(); defer { fetchLock.unlock() }; _isFetchingStatus = newValue } + }
36-51: Inconsistent logging: some error paths still useprint()instead ofAppLogger.The
fetchDatamethod now usesAppLogger.shared.log, but other methods in this file (lines 36, 48, 51, 87, 99, etc.) still useprint(). Consider standardizing onAppLoggerthroughout for unified logging.NetbirdNetworkExtension/PacketTunnelProvider.swift (1)
62-96: Inconsistent logging: manyprint()statements remain.Lines 62, 95, 170, 220, 226, 261, 267, 277, 285, 299, 302, 319, 328, 335, 341, 349 still use
print()while network-related messages useAppLogger.shared.log. Consider migrating all logging toAppLoggerfor unified log collection.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
NetBird.xcodeproj/project.pbxproj(9 hunks)NetBird/Source/App/Views/AdvancedView.swift(1 hunks)NetbirdKit/NetworkExtensionAdapter.swift(2 hunks)NetbirdNetworkExtension/NetBirdAdapter.swift(3 hunks)NetbirdNetworkExtension/PacketTunnelProvider.swift(3 hunks)README.md(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- NetbirdNetworkExtension/NetBirdAdapter.swift
🧰 Additional context used
🧬 Code graph analysis (3)
NetBird/Source/App/Views/AdvancedView.swift (1)
NetbirdKit/AppLogger.swift (3)
log(80-90)getGoLogFileURL(139-156)getLogFileURL(131-137)
NetbirdKit/NetworkExtensionAdapter.swift (1)
NetbirdKit/AppLogger.swift (1)
log(80-90)
NetbirdNetworkExtension/PacketTunnelProvider.swift (2)
NetbirdKit/NetworkExtensionAdapter.swift (2)
stop(136-138)start(45-53)NetbirdNetworkExtension/NetBirdAdapter.swift (2)
stop(133-149)start(106-122)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build for Simulator
- GitHub Check: Build iOS App
🔇 Additional comments (5)
NetBird.xcodeproj/project.pbxproj (1)
828-862: Build configuration updates look appropriate.Version bump to 0.0.14 with build number 7 and enabling Mac Designed for iPhone/iPad support aligns with the bug fix release. AppLogger.swift is correctly included in both targets' build phases for unified logging across the app and network extension.
NetbirdKit/NetworkExtensionAdapter.swift (1)
239-276: Status polling guard implementation looks correct.The guard pattern effectively prevents concurrent
fetchDatacalls. Thedeferstatement properly resets the flag on all exit paths within the closure, and manual resets handle the error paths before the closure is invoked.NetbirdNetworkExtension/PacketTunnelProvider.swift (2)
32-35: Optional Firebase configuration is a good improvement.Gracefully handling the absence of
GoogleService-Info.plistallows the app to function without Firebase, improving flexibility for local development and testing.
139-160: Restart sequence implementation addresses the original bug.The staged restart with
isRestartingflag and completion-based stop/start flow properly handles rapid state changes during network type switches, preventing the UI from getting stuck in "Connecting..." state.README.md (1)
92-94: Firebase configuration documentation aligns with code changes.The new section accurately describes the optional Firebase setup, matching the code changes in
NetBirdApp.swiftandPacketTunnelProvider.swiftthat gracefully handle missingGoogleService-Info.plist.
| func shareButtonTapped() { | ||
| let fileManager = FileManager.default | ||
| guard let groupURL = fileManager.containerURL(forSecurityApplicationGroupIdentifier: "group.io.netbird.app") else { | ||
| print("Failed to retrieve the group URL") | ||
| guard let documentsDir = getDocumentsDirectory() else { | ||
| AppLogger.shared.log("Failed to get documents directory") | ||
| return | ||
| } | ||
|
|
||
| let logURL = groupURL.appendingPathComponent("logfile.log") | ||
| var filesToShare: [URL] = [] | ||
|
|
||
| do { | ||
| let logData = try String(contentsOf: logURL, encoding: .utf8) | ||
| let fileName = "netbird-log.txt" | ||
| guard let filePath = getDocumentsDirectory()?.appendingPathComponent(fileName) else { | ||
| print("Failed to get file path") | ||
| return | ||
| // Export Go SDK logs | ||
| if let goLogURL = AppLogger.getGoLogFileURL() { | ||
| do { | ||
| let goLogData = try String(contentsOf: goLogURL, encoding: .utf8) | ||
| let goLogPath = documentsDir.appendingPathComponent("netbird-engine.log") | ||
| try goLogData.write(to: goLogPath, atomically: true, encoding: .utf8) | ||
| filesToShare.append(goLogPath) | ||
| } catch { | ||
| AppLogger.shared.log("Failed to read Go log data: \(error)") | ||
| } | ||
|
|
||
| } | ||
|
|
||
| // Export Swift logs | ||
| if let swiftLogURL = AppLogger.getLogFileURL() { | ||
| do { | ||
| try logData.write(to: filePath, atomically: true, encoding: .utf8) | ||
|
|
||
| let activityViewController = UIActivityViewController(activityItems: [filePath], applicationActivities: nil) | ||
|
|
||
| activityViewController.excludedActivityTypes = [ | ||
| .assignToContact, | ||
| .saveToCameraRoll | ||
| ] | ||
|
|
||
| if let windowScene = UIApplication.shared.connectedScenes.first as? UIWindowScene, | ||
| let rootViewController = windowScene.windows.first?.rootViewController { | ||
| rootViewController.present(activityViewController, animated: true, completion: nil) | ||
| } | ||
| let swiftLogData = try String(contentsOf: swiftLogURL, encoding: .utf8) | ||
| let swiftLogPath = documentsDir.appendingPathComponent("netbird-app.log") | ||
| try swiftLogData.write(to: swiftLogPath, atomically: true, encoding: .utf8) | ||
| filesToShare.append(swiftLogPath) | ||
| } catch { | ||
| print("Failed to write to file: \(error.localizedDescription)") | ||
| AppLogger.shared.log("Failed to read Swift log data: \(error)") | ||
| } | ||
| } catch { | ||
| print("Failed to read log data: \(error)") | ||
| } | ||
|
|
||
| guard !filesToShare.isEmpty else { | ||
| AppLogger.shared.log("No log files to share") | ||
| return | ||
| } | ||
|
|
||
| let activityViewController = UIActivityViewController(activityItems: filesToShare, applicationActivities: nil) | ||
|
|
||
| activityViewController.excludedActivityTypes = [ | ||
| .assignToContact, | ||
| .saveToCameraRoll | ||
| ] | ||
|
|
||
| if let windowScene = UIApplication.shared.connectedScenes.first as? UIWindowScene, | ||
| let rootViewController = windowScene.windows.first?.rootViewController { | ||
| rootViewController.present(activityViewController, animated: true, completion: nil) | ||
| } | ||
| } |
There was a problem hiding this comment.
Potential iPad crash: UIActivityViewController needs a popover anchor
On iPad, presenting an activity controller without configuring popoverPresentationController can crash.
let activityViewController = UIActivityViewController(activityItems: filesToShare, applicationActivities: nil)
activityViewController.excludedActivityTypes = [
.assignToContact,
.saveToCameraRoll
]
if let windowScene = UIApplication.shared.connectedScenes.first as? UIWindowScene,
let rootViewController = windowScene.windows.first?.rootViewController {
+ if let popover = activityViewController.popoverPresentationController {
+ popover.sourceView = rootViewController.view
+ popover.sourceRect = CGRect(
+ x: rootViewController.view.bounds.midX,
+ y: rootViewController.view.bounds.midY,
+ width: 0,
+ height: 0
+ )
+ popover.permittedArrowDirections = []
+ }
rootViewController.present(activityViewController, animated: true, completion: nil)
}🤖 Prompt for AI Agents
In NetBird/Source/App/Views/AdvancedView.swift around lines 189-237, the
UIActivityViewController is presented without configuring its
popoverPresentationController which can crash on iPad; set
activityViewController.popoverPresentationController?.sourceView and sourceRect
(or barButtonItem if you have one) before presenting, e.g. use the
rootViewController.view as the sourceView and rootViewController.view.bounds (or
a safe CGRect) as sourceRect, and optionally set permittedArrowDirections to
.any to ensure the activity controller is anchored properly on iPad.
Avoid persisting logs in Documents/ (backup/PII retention); prefer temp/caches + cleanup
Exporting logs into Documents makes them long-lived and potentially iCloud-backed-up. Consider FileManager.default.temporaryDirectory (or caches) and delete the exported files after the share completes.
- guard let documentsDir = getDocumentsDirectory() else {
- AppLogger.shared.log("Failed to get documents directory")
- return
- }
+ let exportDir = FileManager.default.temporaryDirectory
+ .appendingPathComponent("netbird-log-export", isDirectory: true)
+ do { try FileManager.default.createDirectory(at: exportDir, withIntermediateDirectories: true) }
+ catch {
+ AppLogger.shared.log("Failed to create export directory: \(error)")
+ return
+ }
var filesToShare: [URL] = []
// Export Go SDK logs
if let goLogURL = AppLogger.getGoLogFileURL() {
do {
- let goLogData = try String(contentsOf: goLogURL, encoding: .utf8)
- let goLogPath = documentsDir.appendingPathComponent("netbird-engine.log")
- try goLogData.write(to: goLogPath, atomically: true, encoding: .utf8)
+ let goLogPath = exportDir.appendingPathComponent("netbird-engine.log")
+ try? FileManager.default.removeItem(at: goLogPath)
+ try FileManager.default.copyItem(at: goLogURL, to: goLogPath)
filesToShare.append(goLogPath)
} catch {
- AppLogger.shared.log("Failed to read Go log data: \(error)")
+ AppLogger.shared.log("Failed to export Go log: \(error)")
}
}
// Export Swift logs
if let swiftLogURL = AppLogger.getLogFileURL() {
do {
- let swiftLogData = try String(contentsOf: swiftLogURL, encoding: .utf8)
- let swiftLogPath = documentsDir.appendingPathComponent("netbird-app.log")
- try swiftLogData.write(to: swiftLogPath, atomically: true, encoding: .utf8)
+ let swiftLogPath = exportDir.appendingPathComponent("netbird-app.log")
+ try? FileManager.default.removeItem(at: swiftLogPath)
+ try FileManager.default.copyItem(at: swiftLogURL, to: swiftLogPath)
filesToShare.append(swiftLogPath)
} catch {
- AppLogger.shared.log("Failed to read Swift log data: \(error)")
+ AppLogger.shared.log("Failed to export Swift log: \(error)")
}
}
guard !filesToShare.isEmpty else {
AppLogger.shared.log("No log files to share")
return
}
let activityViewController = UIActivityViewController(activityItems: filesToShare, applicationActivities: nil)
+ activityViewController.completionWithItemsHandler = { _, _, _, _ in
+ try? FileManager.default.removeItem(at: exportDir)
+ }🤖 Prompt for AI Agents
In NetBird/Source/App/Views/AdvancedView.swift around lines 189 to 237, the code
writes exported logs into the Documents directory which can be long-lived and
iCloud-backed; change it to write the temporary export files into
FileManager.default.temporaryDirectory (or Caches) instead and ensure they are
removed after the share completes. Specifically: replace getDocumentsDirectory
usage with a temp directory (create a unique temp subdirectory or filenames),
write the log files there, present the UIActivityViewController as before, and
in the activityViewController.completionWithItemsHandler (or completion handler)
remove the exported temp files (and the temp directory) using FileManager,
handling errors but not blocking the UI; also ensure you still log read/write
errors and guard that cleanup runs whether the share succeeded or was cancelled.
| } else { | ||
| // Note: isRestarting is already cleared by onConnected() callback | ||
| self?.adapter.isRestarting = false | ||
| AppLogger.shared.log("restartClient: start completed successfully") |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Misleading comment about isRestarting reset.
The comment states "isRestarting is already cleared by onConnected() callback" but then immediately sets isRestarting = false. This is confusing - either the comment is outdated (if onConnected doesn't clear it) or the explicit reset is redundant (if onConnected does clear it).
Remove the misleading comment or clarify the actual behavior:
} else {
- // Note: isRestarting is already cleared by onConnected() callback
self?.adapter.isRestarting = false
AppLogger.shared.log("restartClient: start completed successfully")
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| } else { | |
| // Note: isRestarting is already cleared by onConnected() callback | |
| self?.adapter.isRestarting = false | |
| AppLogger.shared.log("restartClient: start completed successfully") | |
| } else { | |
| self?.adapter.isRestarting = false | |
| AppLogger.shared.log("restartClient: start completed successfully") |
🤖 Prompt for AI Agents
In NetbirdNetworkExtension/PacketTunnelProvider.swift around lines 153 to 156,
the inline comment claims "isRestarting is already cleared by onConnected()
callback" while the code immediately sets self?.adapter.isRestarting = false,
creating confusion about which actually clears the flag; remove the misleading
comment or update it to reflect the truth (either state that onConnected no
longer clears isRestarting and the reset here is required, or state that the
explicit reset is redundant and can be removed), and ensure the remaining
comment accurately documents where and why isRestarting is cleared.
| - Xcode 16.0+ | ||
| - Go 1.23+ | ||
| - gomobile |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check Go and Xcode versions in CI workflow files
echo "=== Checking CI workflow configurations ==="
find .github/workflows -type f \( -name "*.yml" -o -name "*.yaml" \) 2>/dev/null | while read file; do
echo "--- $file ---"
grep -E "(go-version|xcode|Go|Xcode)" "$file" | head -20
doneRepository: netbirdio/ios-client
Length of output: 856
Update README version requirements to match CI configurations.
The README specifies Go 1.23+ and Xcode 16.0+, but CI workflows test against Go 1.24 and Xcode 16.1. Update the README to reflect the actual tested versions to prevent compatibility issues for developers using lower versions.
🤖 Prompt for AI Agents
In README.md around lines 64 to 66, update the listed tool version requirements
to match CI: change Go 1.23+ to Go 1.24+ and Xcode 16.0+ to Xcode 16.1+ (leave
gomobile unchanged); ensure the version numbers are updated wherever else they
appear in the README so documentation matches the CI workflows.
Fix UI stuck in connecting state during network type switches
When switching between wifi and cellular, the Go engine fires rapid state
changes (connecting → disconnecting → connected) that confused the UI
animation state machine, causing it to get stuck in "Connecting..." state.
Changes:
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.