Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 17 additions & 5 deletions payjoin-ffi/src/receive/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,15 @@ impl ReceiverCreateRequestError {
#[derive(Debug, thiserror::Error, uniffi::Error)]
#[error(transparent)]
pub enum ReceiverPersistedError {
/// rust-payjoin receiver error
/// Transient rust-payjoin receiver error. No state transition was
/// persisted: the session remains in its previous state and the caller
/// may retry the same operation later.
#[error(transparent)]
Receiver(ReceiverError),
Transient(ReceiverError),
/// Fatal rust-payjoin receiver error. The session was closed, or
/// transitioned to a terminal error state, and must not be retried.
#[error(transparent)]
Fatal(ReceiverError),
/// Storage error that could occur at application storage layer
#[error(transparent)]
Storage(Arc<ImplementationError>),
Expand All @@ -87,12 +93,18 @@ macro_rules! impl_persisted_error_from {
if let Some(storage_err) = err.storage_error() {
return ReceiverPersistedError::from(ImplementationError::new(storage_err));
}
return ReceiverPersistedError::Receiver(ReceiverError::Unexpected);
return ReceiverPersistedError::Fatal(ReceiverError::Unexpected);
}
let transient = err.is_transient();
if let Some(api_err) = err.api_error() {
return ReceiverPersistedError::Receiver($receiver_arm(api_err));
let receiver_error = $receiver_arm(api_err);
return if transient {
ReceiverPersistedError::Transient(receiver_error)
} else {
ReceiverPersistedError::Fatal(receiver_error)
};
}
ReceiverPersistedError::Receiver(ReceiverError::Unexpected)
ReceiverPersistedError::Fatal(ReceiverError::Unexpected)
}
}
};
Expand Down
39 changes: 39 additions & 0 deletions payjoin/src/core/persist.rs
Original file line number Diff line number Diff line change
Expand Up @@ -760,6 +760,28 @@ where
_ => None,
}
}

/// Whether this error was transient.
///
/// A transient error means no state transition was persisted: the session
/// remains in its previous state and the caller may retry the same
/// operation later. Returns `false` for storage errors; use
/// [`Self::storage_error_ref`] to distinguish those.
pub fn is_transient(&self) -> bool {
matches!(self.0, InternalPersistedError::Api(ApiError::Transient(_)))
}

/// Whether this error was fatal.
///
/// A fatal error means the session was closed, or transitioned to a
/// terminal error state, and must not be retried. Returns `false` for
/// storage errors; use [`Self::storage_error_ref`] to distinguish those.
pub fn is_fatal(&self) -> bool {
matches!(
self.0,
InternalPersistedError::Api(ApiError::Fatal(_) | ApiError::FatalWithState(_, _))
)
}
Comment on lines +779 to +784

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems redundant IMO, If is_transient() returns a bool why do we need an additional method to return the inverse?

@chavic chavic Jul 9, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

error is_transient() is_fatal() !is_transient()
Api Transient true false false
Api Fatal / FatalWithState false true true
Storage false false true ← the trap

@benalleng This is kinda how I'm thinking about it, but there's probably a way to simply... You've basically given a litmus test for the API shape in a way

}

impl<ApiError: std::error::Error, StorageError: std::error::Error, ErrorState: fmt::Debug>
Expand Down Expand Up @@ -1592,18 +1614,35 @@ mod tests {
);
assert!(storage_error.storage_error_ref().is_some());
assert!(storage_error.api_error_ref().is_none());
assert!(!storage_error.is_transient());
assert!(!storage_error.is_fatal());

// Test Internal API error cases
let fatal_error = PersistedError::<InMemoryTestError, InMemoryTestError>(
InternalPersistedError::Api(ApiError::Fatal(api_err.clone())),
);
assert!(fatal_error.storage_error_ref().is_none());
assert!(fatal_error.api_error_ref().is_some());
assert!(!fatal_error.is_transient());
assert!(fatal_error.is_fatal());

let transient_error = PersistedError::<InMemoryTestError, InMemoryTestError>(
InternalPersistedError::Api(ApiError::Transient(api_err.clone())),
);
assert!(transient_error.storage_error_ref().is_none());
assert!(transient_error.api_error_ref().is_some());
assert!(transient_error.is_transient());
assert!(!transient_error.is_fatal());

let fatal_with_state_error = PersistedError::<InMemoryTestError, InMemoryTestError, String>(
InternalPersistedError::Api(ApiError::FatalWithState(
api_err.clone(),
"Error state".to_string(),
)),
);
assert!(fatal_with_state_error.storage_error_ref().is_none());
assert!(fatal_with_state_error.api_error_ref().is_some());
assert!(!fatal_with_state_error.is_transient());
assert!(fatal_with_state_error.is_fatal());
}
}
Loading