fix(apple): don't panic in FFI functions (#8202)

Now that we have error reporting via Sentry in Swift-land as well, we
can handle errors in the FFI layer more gracefully and return them to
Swift.

---------

Signed-off-by: Jamil <jamilbk@users.noreply.github.com>
Co-authored-by: Jamil <jamilbk@users.noreply.github.com>
This commit is contained in:
Thomas Eizinger
2025-02-20 11:51:56 +11:00
committed by GitHub
parent 2dae8bd656
commit cad84922db
5 changed files with 55 additions and 27 deletions

View File

@@ -73,11 +73,11 @@ mod ffi {
//
// `dns_servers` must not have any IPv6 scopes
// <https://github.com/firezone/firezone/issues/4350>
#[swift_bridge(swift_name = "setDns")]
fn set_dns(&mut self, dns_servers: String);
#[swift_bridge(swift_name = "setDns", return_with = err_to_string)]
fn set_dns(&mut self, dns_servers: String) -> Result<(), String>;
#[swift_bridge(swift_name = "setDisabledResources")]
fn set_disabled_resources(&mut self, disabled_resources: String);
#[swift_bridge(swift_name = "setDisabledResources", return_with = err_to_string)]
fn set_disabled_resources(&mut self, disabled_resources: String) -> Result<(), String>;
fn disconnect(self);
}
@@ -300,18 +300,26 @@ impl WrappedSession {
self.inner.reset()
}
fn set_dns(&mut self, dns_servers: String) {
let dns_servers =
serde_json::from_str(&dns_servers).expect("Failed to deserialize DNS servers");
fn set_dns(&mut self, dns_servers: String) -> Result<()> {
tracing::debug!(%dns_servers);
self.inner.set_dns(dns_servers)
let dns_servers = serde_json::from_str(&dns_servers)
.context("Failed to deserialize DNS servers from JSON")?;
self.inner.set_dns(dns_servers);
Ok(())
}
fn set_disabled_resources(&mut self, disabled_resources: String) {
let disabled_resources = serde_json::from_str(&disabled_resources)
.expect("Failed to deserialize disabled resources");
fn set_disabled_resources(&mut self, disabled_resources: String) -> Result<()> {
tracing::debug!(%disabled_resources);
self.inner.set_disabled_resources(disabled_resources)
let disabled_resources = serde_json::from_str(&disabled_resources)
.context("Failed to deserialize disabled resources from JSON")?;
self.inner.set_disabled_resources(disabled_resources);
Ok(())
}
fn disconnect(mut self) {
@@ -320,12 +328,8 @@ impl WrappedSession {
}
}
fn err_to_string(result: Result<WrappedSession>) -> Result<WrappedSession, String> {
result.map_err(|e| {
tracing::error!("Failed to create session: {e:#}");
format!("{e:#}")
})
fn err_to_string<T>(result: Result<T>) -> Result<T, String> {
result.map_err(|e| format!("{e:#}"))
}
/// Installs the `ring` crypto provider for rustls.

View File

@@ -20,6 +20,10 @@ enum AdapterError: Error {
/// connlib failed to start
case connlibConnectError(String)
case setDnsError(String)
case setDisabledResourcesError(String)
var localizedDescription: String {
switch self {
case .invalidSession(let session):
@@ -27,6 +31,10 @@ enum AdapterError: Error {
return message
case .connlibConnectError(let error):
return "connlib failed to start: \(error)"
case .setDnsError(let error):
return "failed to set new DNS serversn: \(error)"
case .setDisabledResourcesError(let error):
return "failed to set new disabled resources: \(error)"
}
}
}
@@ -136,7 +144,13 @@ class Adapter {
let encoded = try? JSONEncoder().encode(resolvers),
let jsonResolvers = String(data: encoded, encoding: .utf8)?.intoRustString() {
session?.setDns(jsonResolvers)
do {
try session?.setDns(jsonResolvers)
} catch let error {
// `toString` needed to deep copy the string and avoid a possible dangling pointer
let msg = (error as? RustString)?.toString() ?? "Unknown error"
Log.error(AdapterError.setDnsError(msg))
}
// Update our state tracker
self.lastFetchedResolvers = resolvers
@@ -307,7 +321,13 @@ class Adapter {
fatalError("Should be able to encode 'disablingResources'")
}
session?.setDisabledResources(toSet)
do {
try session?.setDisabledResources(toSet)
} catch let error {
// `toString` needed to deep copy the string and avoid a possible dangling pointer
let msg = (error as? RustString)?.toString() ?? "Unknown error"
Log.error(AdapterError.setDisabledResourcesError(msg))
}
}
}

View File

@@ -13,8 +13,8 @@ void* __swift_bridge__$Vec_WrappedSession$as_ptr(void* vec_ptr);
struct __private__ResultPtrAndPtr __swift_bridge__$WrappedSession$connect(void* api_url, void* token, void* device_id, void* account_slug, void* device_name_override, void* os_version_override, void* log_dir, void* log_filter, void* callback_handler, void* device_info);
void __swift_bridge__$WrappedSession$reset(void* self);
void __swift_bridge__$WrappedSession$set_dns(void* self, void* dns_servers);
void __swift_bridge__$WrappedSession$set_disabled_resources(void* self, void* disabled_resources);
void* __swift_bridge__$WrappedSession$set_dns(void* self, void* dns_servers);
void* __swift_bridge__$WrappedSession$set_disabled_resources(void* self, void* disabled_resources);
void __swift_bridge__$WrappedSession$disconnect(void* self);

View File

@@ -46,12 +46,12 @@ extension WrappedSessionRefMut {
__swift_bridge__$WrappedSession$reset(ptr)
}
public func setDns<GenericIntoRustString: IntoRustString>(_ dns_servers: GenericIntoRustString) {
__swift_bridge__$WrappedSession$set_dns(ptr, { let rustString = dns_servers.intoRustString(); rustString.isOwned = false; return rustString.ptr }())
public func setDns<GenericIntoRustString: IntoRustString>(_ dns_servers: GenericIntoRustString) throws -> () {
try { let val = __swift_bridge__$WrappedSession$set_dns(ptr, { let rustString = dns_servers.intoRustString(); rustString.isOwned = false; return rustString.ptr }()); if val != nil { throw RustString(ptr: val!) } else { return } }()
}
public func setDisabledResources<GenericIntoRustString: IntoRustString>(_ disabled_resources: GenericIntoRustString) {
__swift_bridge__$WrappedSession$set_disabled_resources(ptr, { let rustString = disabled_resources.intoRustString(); rustString.isOwned = false; return rustString.ptr }())
public func setDisabledResources<GenericIntoRustString: IntoRustString>(_ disabled_resources: GenericIntoRustString) throws -> () {
try { let val = __swift_bridge__$WrappedSession$set_disabled_resources(ptr, { let rustString = disabled_resources.intoRustString(); rustString.isOwned = false; return rustString.ptr }()); if val != nil { throw RustString(ptr: val!) } else { return } }()
}
}
public class WrappedSessionRef {

View File

@@ -19,7 +19,11 @@ export default function Apple() {
return (
<Entries downloadLinks={downloadLinks} title="macOS / iOS">
{/* When you cut a release, remove any solved issues from the "known issues" lists over in `client-apps`. This must not be done when the issue's PR merges. */}
<Unreleased></Unreleased>
<Unreleased>
<ChangeItem pull="8202">
Fixes a crash that occurred if the system reports invalid DNS servers.
</ChangeItem>
</Unreleased>
<Entry version="1.4.3" date={new Date("2025-02-16")}>
<ChangeItem pull="8122">
Fixes a rare crash that could occur when dismissing the update