Skip to content

Commit 19354bc

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 d216d21 commit 19354bc

File tree

7 files changed

+323
-48
lines changed

7 files changed

+323
-48
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**: 54
7+
**Total Functions**: 56
88

99
## Table of Contents
1010

@@ -87,12 +87,14 @@ Functions: 2
8787

8888
### Event Callbacks
8989

90-
Functions: 4
90+
Functions: 6
9191

9292
| Function | Description | Module |
9393
|----------|-------------|--------|
94+
| `dash_spv_ffi_client_clear_client_error_callback` | Clear the client error callback | client |
9495
| `dash_spv_ffi_client_clear_network_event_callbacks` | Clear network event callbacks | client |
9596
| `dash_spv_ffi_client_clear_progress_callback` | Clear progress callback | client |
97+
| `dash_spv_ffi_client_set_client_error_callback` | Set a callback for fatal client errors (start failure, sync thread crash) | client |
9698
| `dash_spv_ffi_client_set_network_event_callbacks` | Set network event callbacks for push-based event notifications | client |
9799
| `dash_spv_ffi_client_set_progress_callback` | Set progress callback for sync progress updates | client |
98100

@@ -621,6 +623,22 @@ This function is unsafe because: - The caller must ensure all pointers are valid
621623
622624
### Event Callbacks - Detailed
623625
626+
#### `dash_spv_ffi_client_clear_client_error_callback`
627+
628+
```c
629+
dash_spv_ffi_client_clear_client_error_callback(client: *mut FFIDashSpvClient,) -> i32
630+
```
631+
632+
**Description:**
633+
Clear the client error callback. # Safety - `client` must be a valid, non-null pointer to an `FFIDashSpvClient`.
634+
635+
**Safety:**
636+
- `client` must be a valid, non-null pointer to an `FFIDashSpvClient`.
637+
638+
**Module:** `client`
639+
640+
---
641+
624642
#### `dash_spv_ffi_client_clear_network_event_callbacks`
625643

626644
```c
@@ -653,6 +671,22 @@ Clear progress callback. # Safety - `client` must be a valid, non-null pointer
653671

654672
---
655673

674+
#### `dash_spv_ffi_client_set_client_error_callback`
675+
676+
```c
677+
dash_spv_ffi_client_set_client_error_callback(client: *mut FFIDashSpvClient, callback: FFIClientErrorCallback,) -> i32
678+
```
679+
680+
**Description:**
681+
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.
682+
683+
**Safety:**
684+
- `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.
685+
686+
**Module:** `client`
687+
688+
---
689+
656690
#### `dash_spv_ffi_client_set_network_event_callbacks`
657691
658692
```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
*/
@@ -752,6 +769,27 @@ int32_t dash_spv_ffi_client_set_progress_callback(struct FFIDashSpvClient *clien
752769
*/
753770
int32_t dash_spv_ffi_client_clear_progress_callback(struct FFIDashSpvClient *client) ;
754771

772+
/**
773+
* Set a callback for fatal client errors (start failure, sync thread crash).
774+
*
775+
* # Safety
776+
* - `client` must be a valid, non-null pointer to an `FFIDashSpvClient`.
777+
* - The `callback` struct and its `user_data` must remain valid until the callback is cleared.
778+
* - The callback must be thread-safe as it may be called from a background thread.
779+
*/
780+
781+
int32_t dash_spv_ffi_client_set_client_error_callback(struct FFIDashSpvClient *client,
782+
struct FFIClientErrorCallback callback)
783+
;
784+
785+
/**
786+
* Clear the client error callback.
787+
*
788+
* # Safety
789+
* - `client` must be a valid, non-null pointer to an `FFIDashSpvClient`.
790+
*/
791+
int32_t dash_spv_ffi_client_clear_client_error_callback(struct FFIDashSpvClient *client) ;
792+
755793
struct FFIClientConfig *dash_spv_ffi_config_new(FFINetwork network) ;
756794

757795
struct FFIClientConfig *dash_spv_ffi_config_mainnet(void) ;

dash-spv-ffi/src/callbacks.rs

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -569,6 +569,47 @@ impl Default for FFIWalletEventCallbacks {
569569
}
570570
}
571571

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

dash-spv-ffi/src/client.rs

Lines changed: 60 additions & 32 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;
@@ -130,6 +130,7 @@ pub struct FFIDashSpvClient {
130130
network_event_callbacks: Arc<Mutex<Option<FFINetworkEventCallbacks>>>,
131131
wallet_event_callbacks: Arc<Mutex<Option<FFIWalletEventCallbacks>>>,
132132
progress_callback: Arc<Mutex<Option<FFIProgressCallback>>>,
133+
client_error_callback: Arc<Mutex<Option<FFIClientErrorCallback>>>,
133134
}
134135

135136
/// Create a new SPV client and return an opaque pointer.
@@ -189,6 +190,7 @@ pub unsafe extern "C" fn dash_spv_ffi_client_new(
189190
network_event_callbacks: Arc::new(Mutex::new(None)),
190191
wallet_event_callbacks: Arc::new(Mutex::new(None)),
191192
progress_callback: Arc::new(Mutex::new(None)),
193+
client_error_callback: Arc::new(Mutex::new(None)),
192194
};
193195
Box::into_raw(Box::new(ffi_client))
194196
}
@@ -380,34 +382,6 @@ pub unsafe extern "C" fn dash_spv_ffi_client_run(client: *mut FFIDashSpvClient)
380382

381383
tracing::info!("dash_spv_ffi_client_run: starting client");
382384

383-
// Start the client first
384-
let inner = client.inner.clone();
385-
let start_result = client.runtime.block_on(async {
386-
let mut spv_client = {
387-
let mut guard = inner.lock().unwrap();
388-
match guard.take() {
389-
Some(c) => c,
390-
None => {
391-
return Err(dash_spv::SpvError::Storage(dash_spv::StorageError::NotFound(
392-
"Client not initialized".to_string(),
393-
)))
394-
}
395-
}
396-
};
397-
let res = spv_client.start().await;
398-
let mut guard = inner.lock().unwrap();
399-
*guard = Some(spv_client);
400-
res
401-
});
402-
403-
if let Err(e) = start_result {
404-
tracing::error!("dash_spv_ffi_client_run: start failed: {}", e);
405-
set_last_error(&e.to_string());
406-
return FFIErrorCode::from(e) as i32;
407-
}
408-
409-
tracing::info!("dash_spv_ffi_client_run: client started, setting up event monitoring");
410-
411385
// Get event subscriptions before taking the client for the sync thread.
412386
// The sync thread needs exclusive access, so we must subscribe first.
413387
let inner = client.inner.clone();
@@ -487,6 +461,8 @@ pub unsafe extern "C" fn dash_spv_ffi_client_run(client: *mut FFIDashSpvClient)
487461

488462
tracing::info!("dash_spv_ffi_client_run: spawning sync thread");
489463

464+
let error_callback = client.client_error_callback.clone();
465+
490466
// Now take the client for the sync thread
491467
let spv_client = {
492468
let mut guard = inner.lock().unwrap();
@@ -506,7 +482,19 @@ pub unsafe extern "C" fn dash_spv_ffi_client_run(client: *mut FFIDashSpvClient)
506482

507483
let mut spv_client = spv_client;
508484

509-
tracing::debug!("Sync thread: got client, starting monitor_network");
485+
if let Err(e) = spv_client.start().await {
486+
tracing::error!("Sync thread: client start error: {}", e);
487+
let guard = error_callback.lock().unwrap();
488+
if let Some(cb) = guard.as_ref() {
489+
cb.dispatch(&e.to_string());
490+
}
491+
drop(guard);
492+
let mut guard = inner.lock().unwrap();
493+
*guard = Some(spv_client);
494+
return;
495+
}
496+
497+
tracing::debug!("Sync thread: starting monitor_network");
510498

511499
let (_command_sender, command_receiver) = tokio::sync::mpsc::unbounded_channel();
512500
let run_token = shutdown_token.clone();
@@ -536,6 +524,11 @@ pub unsafe extern "C" fn dash_spv_ffi_client_run(client: *mut FFIDashSpvClient)
536524

537525
if let Err(e) = result {
538526
tracing::error!("Sync thread: sync error: {}", e);
527+
let guard = error_callback.lock().unwrap();
528+
if let Some(cb) = guard.as_ref() {
529+
cb.dispatch(&e.to_string());
530+
}
531+
drop(guard);
539532
}
540533

541534
tracing::debug!("Sync thread: putting client back");
@@ -1071,3 +1064,38 @@ pub unsafe extern "C" fn dash_spv_ffi_client_clear_progress_callback(
10711064

10721065
FFIErrorCode::Success as i32
10731066
}
1067+
1068+
/// Set a callback for fatal client errors (start failure, sync thread crash).
1069+
///
1070+
/// # Safety
1071+
/// - `client` must be a valid, non-null pointer to an `FFIDashSpvClient`.
1072+
/// - The `callback` struct and its `user_data` must remain valid until the callback is cleared.
1073+
/// - The callback must be thread-safe as it may be called from a background thread.
1074+
#[no_mangle]
1075+
pub unsafe extern "C" fn dash_spv_ffi_client_set_client_error_callback(
1076+
client: *mut FFIDashSpvClient,
1077+
callback: FFIClientErrorCallback,
1078+
) -> i32 {
1079+
null_check!(client);
1080+
1081+
let client = &(*client);
1082+
*client.client_error_callback.lock().unwrap() = Some(callback);
1083+
1084+
FFIErrorCode::Success as i32
1085+
}
1086+
1087+
/// Clear the client error callback.
1088+
///
1089+
/// # Safety
1090+
/// - `client` must be a valid, non-null pointer to an `FFIDashSpvClient`.
1091+
#[no_mangle]
1092+
pub unsafe extern "C" fn dash_spv_ffi_client_clear_client_error_callback(
1093+
client: *mut FFIDashSpvClient,
1094+
) -> i32 {
1095+
null_check!(client);
1096+
1097+
let client = &(*client);
1098+
*client.client_error_callback.lock().unwrap() = None;
1099+
1100+
FFIErrorCode::Success as i32
1101+
}

0 commit comments

Comments
 (0)