VPN-4586: Return an error when the Nym log file is not created #4122

trojanfoe · 2025-12-04T14:28:07Z

Ticket

JIRA-VPN-4586

Description

On the mobile platform, if the VPN log files fails to open we capture the error for reporting to the user.

Checklist:

Changelog

Screenshots (optional, if UI related)

This change is

pronebird

As we discussed in internal comms. Let's debate in offline before merging this

pronebird

Reviewable status: 0 of 16 files reviewed, 2 unresolved discussions (waiting on @mrkiwi and @trojanfoe)

nym-vpn-core/crates/nym-vpn-lib-uniffi/src/ios.rs line 51 at r2 (raw file):

        if let Some(parent) = path.parent()
            && !parent.exists()
            && let Err(e) = std::fs::create_dir_all(parent)

IIRC create_dir_all() never fails if created directory already exists. Please verify. There is no need to check for parent.exists() neither.

nym-vpn-core/crates/nym-vpn-lib-uniffi/src/ios.rs line 62 at r2 (raw file):

        // Attempt to open the log file for writing
        let file = match OpenOptions::new()

Rewrite with .map_err()?; perhaps? In most cases Ok(x) => x is indication that something isn't right. Presence or return too.

pronebird

Reviewable status: 0 of 16 files reviewed, 3 unresolved discussions (waiting on @mrkiwi and @trojanfoe)

nym-vpn-core/crates/nym-vpn-lib-uniffi/src/ios.rs line 98 at r2 (raw file):

        .try_init()
        .map_err(|err| VpnError::CreateLogFile {
            details: format!("Failed to initialize OsLogger: {err}"),

Please correct the message. We're not initializing OsLogger alone, but tracing log.

pronebird

Reviewable status: 0 of 16 files reviewed, 4 unresolved discussions (waiting on @mrkiwi and @trojanfoe)

nym-vpn-core/crates/nym-vpn-lib-uniffi/src/lib.rs line 224 at r2 (raw file):

    #[cfg(target_os = "ios")]
    let result = ios::init_logs(log_level, path, sentry_monitoring);

Nit: It's possible to wrap this in { .. } and return explicitly without going via result

pronebird

Reviewable status: 0 of 16 files reviewed, 5 unresolved discussions (waiting on @mrkiwi and @trojanfoe)

nym-vpn-core/crates/nym-vpn-lib-uniffi/src/android.rs line 8 at r2 (raw file):

use crate::error::VpnError;

pub(crate) fn init_logs(level: String) -> Result<(), VpnError> {

This method is infallible. Remove the Result

pronebird

Reviewable status: 0 of 16 files reviewed, 6 unresolved discussions (waiting on @mrkiwi and @trojanfoe)

nym-vpn-android/core/src/main/java/net/nymtech/vpn/backend/NymBackend.kt line 134 at r2 (raw file):

			initLogger(storagePath, LOG_LEVEL, config.sentryMonitoringEnabled)
				?: Timber.e("Failed to initialize backed logger")

typo in backend?

pronebird

Reviewable status: 0 of 16 files reviewed, 7 unresolved discussions (waiting on @mrkiwi and @trojanfoe)

nym-vpn-apple/NymMixnetTunnel/PacketTunnelProvider.swift line 135 at r2 (raw file):

        let logPath = LogFileManager.logFileURL(logFileType: .library)?.path()

        Task {

Problem that this makes logging initialization asynchronous. This needs to be synchronous somehow.

On the mobile platform, if the VPN log files fails to open, then it's considered a fatal error.

pronebird

Reviewable status: 0 of 16 files reviewed, 8 unresolved discussions (waiting on @mrkiwi and @trojanfoe)

nym-vpn-core/crates/nym-firewall/src/net.rs line 29 at r2 (raw file):

/// When "allow local network" is enabled the app will allow traffic to these networks.
#[cfg_attr(
    any(target_os = "windows", target_os = "android", target_os = "ios"),

We don't compile nor even reference nym-firewall on mobile, please undo the change.

pronebird

@pronebird reviewed 3 of 10 files at r1, 13 of 13 files at r2, all commit messages.
Reviewable status: all files reviewed, 7 unresolved discussions (waiting on @mrkiwi and @trojanfoe)

trojanfoe

Reviewable status: all files reviewed, 6 unresolved discussions (waiting on @mrkiwi and @pronebird)

nym-vpn-core/crates/nym-firewall/src/net.rs line 29 at r2 (raw file):