Skip to content

Commit 5f664d3

Browse files
committed
feat: report/propagate client start/run failures
We currently just silently ignore any failure inside the sync coordinator or `monitor_network` thread. This PR propagates them and also adds a FFI callback structure which can be set to receive notifications about errors inside the SPV client.
1 parent 9c41375 commit 5f664d3

File tree

7 files changed

+311
-18
lines changed

7 files changed

+311
-18
lines changed

dash-spv-ffi/FFI_API.md

Lines changed: 36 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ This document provides a comprehensive reference for all FFI (Foreign Function I
44

55
**Auto-generated**: This documentation is automatically generated from the source code. Do not edit manually.
66

7-
**Total Functions**: 48
7+
**Total Functions**: 50
88

99
## Table of Contents
1010

@@ -94,12 +94,14 @@ Functions: 2
9494

9595
### Event Callbacks
9696

97-
Functions: 4
97+
Functions: 6
9898

9999
| Function | Description | Module |
100100
|----------|-------------|--------|
101+
| `dash_spv_ffi_client_clear_client_error_callback` | Clear the client error callback | client |
101102
| `dash_spv_ffi_client_clear_network_event_callbacks` | Clear network event callbacks | client |
102103
| `dash_spv_ffi_client_clear_progress_callback` | Clear progress callback | client |
104+
| `dash_spv_ffi_client_set_client_error_callback` | Set a callback for fatal client errors (start failure, sync thread crash) | client |
103105
| `dash_spv_ffi_client_set_network_event_callbacks` | Set network event callbacks for push-based event notifications | client |
104106
| `dash_spv_ffi_client_set_progress_callback` | Set progress callback for sync progress updates | client |
105107

@@ -609,6 +611,22 @@ This function is unsafe because: - The caller must ensure all pointers are valid
609611
610612
### Event Callbacks - Detailed
611613
614+
#### `dash_spv_ffi_client_clear_client_error_callback`
615+
616+
```c
617+
dash_spv_ffi_client_clear_client_error_callback(client: *mut FFIDashSpvClient,) -> i32
618+
```
619+
620+
**Description:**
621+
Clear the client error callback. # Safety - `client` must be a valid, non-null pointer to an `FFIDashSpvClient`.
622+
623+
**Safety:**
624+
- `client` must be a valid, non-null pointer to an `FFIDashSpvClient`.
625+
626+
**Module:** `client`
627+
628+
---
629+
612630
#### `dash_spv_ffi_client_clear_network_event_callbacks`
613631

614632
```c
@@ -641,6 +659,22 @@ Clear progress callback. # Safety - `client` must be a valid, non-null pointer
641659

642660
---
643661

662+
#### `dash_spv_ffi_client_set_client_error_callback`
663+
664+
```c
665+
dash_spv_ffi_client_set_client_error_callback(client: *mut FFIDashSpvClient, callback: FFIClientErrorCallback,) -> i32
666+
```
667+
668+
**Description:**
669+
Set a callback for fatal client errors (start failure, sync thread crash). # Safety - `client` must be a valid, non-null pointer to an `FFIDashSpvClient`. - The `callback` struct and its `user_data` must remain valid until the callback is cleared. - The callback must be thread-safe as it may be called from a background thread.
670+
671+
**Safety:**
672+
- `client` must be a valid, non-null pointer to an `FFIDashSpvClient`. - The `callback` struct and its `user_data` must remain valid until the callback is cleared. - The callback must be thread-safe as it may be called from a background thread.
673+
674+
**Module:** `client`
675+
676+
---
677+
644678
#### `dash_spv_ffi_client_set_network_event_callbacks`
645679
646680
```c

dash-spv-ffi/include/dash_spv_ffi.h

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -433,6 +433,23 @@ typedef struct FFIProgressCallback {
433433
void *user_data;
434434
} FFIProgressCallback;
435435

436+
/**
437+
* Callback for fatal client errors (e.g. start failure, monitor thread crash).
438+
*
439+
* The `error` string pointer is borrowed and only valid for the duration
440+
* of the callback. Callers must copy the string if they need to retain it
441+
* after the callback returns.
442+
*/
443+
typedef void (*OnClientErrorCallback)(const char *error, void *user_data);
444+
445+
/**
446+
* Client error callback configuration.
447+
*/
448+
typedef struct FFIClientErrorCallback {
449+
OnClientErrorCallback on_error;
450+
void *user_data;
451+
} FFIClientErrorCallback;
452+
436453
/**
437454
* FFIResult type for error handling
438455
*/
@@ -683,6 +700,27 @@ int32_t dash_spv_ffi_client_set_progress_callback(struct FFIDashSpvClient *clien
683700
*/
684701
int32_t dash_spv_ffi_client_clear_progress_callback(struct FFIDashSpvClient *client) ;
685702

703+
/**
704+
* Set a callback for fatal client errors (start failure, sync thread crash).
705+
*
706+
* # Safety
707+
* - `client` must be a valid, non-null pointer to an `FFIDashSpvClient`.
708+
* - The `callback` struct and its `user_data` must remain valid until the callback is cleared.
709+
* - The callback must be thread-safe as it may be called from a background thread.
710+
*/
711+
712+
int32_t dash_spv_ffi_client_set_client_error_callback(struct FFIDashSpvClient *client,
713+
struct FFIClientErrorCallback callback)
714+
;
715+
716+
/**
717+
* Clear the client error callback.
718+
*
719+
* # Safety
720+
* - `client` must be a valid, non-null pointer to an `FFIDashSpvClient`.
721+
*/
722+
int32_t dash_spv_ffi_client_clear_client_error_callback(struct FFIDashSpvClient *client) ;
723+
686724
struct FFIClientConfig *dash_spv_ffi_config_new(FFINetwork network) ;
687725

688726
struct FFIClientConfig *dash_spv_ffi_config_mainnet(void) ;

dash-spv-ffi/src/callbacks.rs

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -575,6 +575,48 @@ impl Default for FFIWalletEventCallbacks {
575575
}
576576
}
577577

578+
// ============================================================================
579+
// FFIClientErrorCallback - Fatal client-level errors
580+
// ============================================================================
581+
582+
/// Callback for fatal client errors (e.g. start failure, monitor thread crash).
583+
///
584+
/// The `error` string pointer is borrowed and only valid for the duration
585+
/// of the callback. Callers must copy the string if they need to retain it
586+
/// after the callback returns.
587+
pub type OnClientErrorCallback =
588+
Option<extern "C" fn(error: *const c_char, user_data: *mut c_void)>;
589+
590+
/// Client error callback configuration.
591+
#[repr(C)]
592+
#[derive(Clone)]
593+
pub struct FFIClientErrorCallback {
594+
pub on_error: OnClientErrorCallback,
595+
pub user_data: *mut c_void,
596+
}
597+
598+
unsafe impl Send for FFIClientErrorCallback {}
599+
unsafe impl Sync for FFIClientErrorCallback {}
600+
601+
impl Default for FFIClientErrorCallback {
602+
fn default() -> Self {
603+
Self {
604+
on_error: None,
605+
user_data: std::ptr::null_mut(),
606+
}
607+
}
608+
}
609+
610+
impl FFIClientErrorCallback {
611+
/// Dispatch a client error to the callback.
612+
pub fn dispatch(&self, error: &str) {
613+
if let Some(cb) = self.on_error {
614+
let c_error = CString::new(error).unwrap_or_default();
615+
cb(c_error.as_ptr(), self.user_data);
616+
}
617+
}
618+
}
619+
578620
impl FFIWalletEventCallbacks {
579621
/// Dispatch a WalletEvent to the appropriate callback.
580622
pub fn dispatch(&self, event: &key_wallet_manager::WalletEvent) {

dash-spv-ffi/src/client.rs

Lines changed: 45 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
use crate::{
2-
null_check, set_last_error, FFIClientConfig, FFIErrorCode, FFINetworkEventCallbacks,
3-
FFIProgressCallback, FFISyncEventCallbacks, FFISyncProgress, FFIWalletEventCallbacks,
4-
FFIWalletManager,
2+
null_check, set_last_error, FFIClientConfig, FFIClientErrorCallback, FFIErrorCode,
3+
FFINetworkEventCallbacks, FFIProgressCallback, FFISyncEventCallbacks, FFISyncProgress,
4+
FFIWalletEventCallbacks, FFIWalletManager,
55
};
66
// Import wallet types from key-wallet-ffi
77
use key_wallet_ffi::FFIWalletManager as KeyWalletFFIWalletManager;
@@ -120,6 +120,7 @@ pub struct FFIDashSpvClient {
120120
network_event_callbacks: Arc<Mutex<Option<FFINetworkEventCallbacks>>>,
121121
wallet_event_callbacks: Arc<Mutex<Option<FFIWalletEventCallbacks>>>,
122122
progress_callback: Arc<Mutex<Option<FFIProgressCallback>>>,
123+
client_error_callback: Arc<Mutex<Option<FFIClientErrorCallback>>>,
123124
}
124125

125126
/// Create a new SPV client and return an opaque pointer.
@@ -179,6 +180,7 @@ pub unsafe extern "C" fn dash_spv_ffi_client_new(
179180
network_event_callbacks: Arc::new(Mutex::new(None)),
180181
wallet_event_callbacks: Arc::new(Mutex::new(None)),
181182
progress_callback: Arc::new(Mutex::new(None)),
183+
client_error_callback: Arc::new(Mutex::new(None)),
182184
};
183185
Box::into_raw(Box::new(ffi_client))
184186
}
@@ -351,13 +353,17 @@ pub unsafe extern "C" fn dash_spv_ffi_client_run(client: *mut FFIDashSpvClient)
351353
));
352354
}
353355

354-
// Spawn the sync monitoring task
356+
let error_callback = client.client_error_callback.clone();
355357
let spv_client = client.inner.clone();
356358
tasks.push(client.runtime.spawn(async move {
357359
tracing::debug!("Sync task: starting run");
358360

359361
if let Err(e) = spv_client.run(shutdown_token).await {
360362
tracing::error!("Sync task: error: {}", e);
363+
let cb = error_callback.lock().unwrap().clone();
364+
if let Some(ref cb) = cb {
365+
cb.dispatch(&e.to_string());
366+
}
361367
}
362368

363369
tracing::debug!("Sync task: exiting");
@@ -720,3 +726,38 @@ pub unsafe extern "C" fn dash_spv_ffi_client_clear_progress_callback(
720726

721727
FFIErrorCode::Success as i32
722728
}
729+
730+
/// Set a callback for fatal client errors (start failure, sync thread crash).
731+
///
732+
/// # Safety
733+
/// - `client` must be a valid, non-null pointer to an `FFIDashSpvClient`.
734+
/// - The `callback` struct and its `user_data` must remain valid until the callback is cleared.
735+
/// - The callback must be thread-safe as it may be called from a background thread.
736+
#[no_mangle]
737+
pub unsafe extern "C" fn dash_spv_ffi_client_set_client_error_callback(
738+
client: *mut FFIDashSpvClient,
739+
callback: FFIClientErrorCallback,
740+
) -> i32 {
741+
null_check!(client);
742+
743+
let client = &(*client);
744+
*client.client_error_callback.lock().unwrap() = Some(callback);
745+
746+
FFIErrorCode::Success as i32
747+
}
748+
749+
/// Clear the client error callback.
750+
///
751+
/// # Safety
752+
/// - `client` must be a valid, non-null pointer to an `FFIDashSpvClient`.
753+
#[no_mangle]
754+
pub unsafe extern "C" fn dash_spv_ffi_client_clear_client_error_callback(
755+
client: *mut FFIDashSpvClient,
756+
) -> i32 {
757+
null_check!(client);
758+
759+
let client = &(*client);
760+
*client.client_error_callback.lock().unwrap() = None;
761+
762+
FFIErrorCode::Success as i32
763+
}

dash-spv-ffi/tests/unit/test_client_lifecycle.rs

Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ mod tests {
99
use key_wallet_ffi::FFINetwork;
1010
use serial_test::serial;
1111
use std::ffi::CString;
12+
use std::sync::mpsc;
13+
use std::sync::{Arc as StdArc, Mutex as StdMutex};
1214
use std::thread;
1315
use std::time::Duration;
1416
use tempfile::TempDir;
@@ -209,6 +211,124 @@ mod tests {
209211
}
210212
}
211213

214+
#[test]
215+
#[serial]
216+
fn test_client_error_callback_fires_on_start_failure() {
217+
let (tx, rx) = mpsc::channel::<String>();
218+
let tx_ptr = Box::into_raw(Box::new(tx));
219+
220+
extern "C" fn on_error(
221+
error: *const std::os::raw::c_char,
222+
user_data: *mut std::os::raw::c_void,
223+
) {
224+
let tx = unsafe { &*(user_data as *const mpsc::Sender<String>) };
225+
let error_str = unsafe { std::ffi::CStr::from_ptr(error) }.to_str().unwrap().to_owned();
226+
let _ = tx.send(error_str);
227+
}
228+
229+
unsafe {
230+
let (config, _temp_dir) = create_test_config_with_dir();
231+
let client = dash_spv_ffi_client_new(config);
232+
assert!(!client.is_null());
233+
234+
let callback = FFIClientErrorCallback {
235+
on_error: Some(on_error),
236+
user_data: tx_ptr as *mut std::os::raw::c_void,
237+
};
238+
let result = dash_spv_ffi_client_set_client_error_callback(client, callback);
239+
assert_eq!(result, FFIErrorCode::Success as i32);
240+
241+
// Call run() twice — the second run's sync thread will call
242+
// start() on the already-running client, triggering "already running"
243+
let run_result = dash_spv_ffi_client_run(client);
244+
assert_eq!(run_result, FFIErrorCode::Success as i32);
245+
246+
// Brief wait for the first run's sync thread to complete start()
247+
thread::sleep(Duration::from_millis(200));
248+
249+
let _run_result2 = dash_spv_ffi_client_run(client);
250+
251+
// Wait for the error callback to fire (with timeout)
252+
let error_msg = rx
253+
.recv_timeout(Duration::from_secs(5))
254+
.expect("Error callback should have been called on start failure");
255+
assert!(
256+
error_msg.contains("already running"),
257+
"Expected 'already running' error, got: {}",
258+
error_msg
259+
);
260+
261+
dash_spv_ffi_client_stop(client);
262+
263+
// Free the sender only after stop has joined all threads,
264+
// so no background thread can call on_error with a dangling user_data.
265+
drop(Box::from_raw(tx_ptr));
266+
267+
dash_spv_ffi_client_destroy(client);
268+
dash_spv_ffi_config_destroy(config);
269+
}
270+
}
271+
272+
#[test]
273+
#[serial]
274+
fn test_client_error_callback_dispatch() {
275+
let error_store: StdArc<StdMutex<Option<String>>> = StdArc::new(StdMutex::new(None));
276+
let error_store_raw = StdArc::into_raw(error_store.clone());
277+
278+
extern "C" fn on_error(
279+
error: *const std::os::raw::c_char,
280+
user_data: *mut std::os::raw::c_void,
281+
) {
282+
assert!(!error.is_null());
283+
let store = unsafe { StdArc::from_raw(user_data as *const StdMutex<Option<String>>) };
284+
let error_str = unsafe { std::ffi::CStr::from_ptr(error) }.to_str().unwrap().to_owned();
285+
*store.lock().unwrap() = Some(error_str);
286+
let _ = StdArc::into_raw(store);
287+
}
288+
289+
let callback = FFIClientErrorCallback {
290+
on_error: Some(on_error),
291+
user_data: error_store_raw as *mut std::os::raw::c_void,
292+
};
293+
294+
callback.dispatch("test error message");
295+
296+
let received = error_store.lock().unwrap();
297+
assert_eq!(received.as_deref(), Some("test error message"));
298+
drop(received);
299+
300+
unsafe { drop(StdArc::from_raw(error_store_raw)) };
301+
}
302+
303+
#[test]
304+
#[serial]
305+
fn test_client_error_callback_null_client() {
306+
unsafe {
307+
let callback = FFIClientErrorCallback {
308+
on_error: None,
309+
user_data: std::ptr::null_mut(),
310+
};
311+
312+
assert_eq!(
313+
dash_spv_ffi_client_set_client_error_callback(std::ptr::null_mut(), callback),
314+
FFIErrorCode::NullPointer as i32
315+
);
316+
317+
assert_eq!(
318+
dash_spv_ffi_client_clear_client_error_callback(std::ptr::null_mut()),
319+
FFIErrorCode::NullPointer as i32
320+
);
321+
}
322+
}
323+
324+
#[test]
325+
#[serial]
326+
fn test_client_error_callback_no_callback_set() {
327+
// Dispatch with no callback set should not panic
328+
let callback = FFIClientErrorCallback::default();
329+
callback.dispatch("should not panic");
330+
}
331+
212332
#[test]
213333
#[serial]
214334
fn test_client_repeated_creation_destruction() {

0 commit comments

Comments
 (0)