Process res error handling (payjoin#1075)

spacebear21 · web-flow · commit e8bb1a7c3b8d · 2025-09-22T17:17:59.000-04:00
diff --git a/payjoin-ffi/src/receive/mod.rs b/payjoin-ffi/src/receive/mod.rs
@@ -316,7 +316,7 @@ pub struct InitializedTransition(
                     payjoin::receive::v2::SessionEvent,
                     payjoin::receive::v2::Receiver<payjoin::receive::v2::UncheckedOriginalPayload>,
                     payjoin::receive::v2::Receiver<payjoin::receive::v2::Initialized>,
-                    payjoin::receive::Error,
+                    payjoin::receive::ProtocolError,
                 >,
             >,
         >,
diff --git a/payjoin/src/core/ohttp.rs b/payjoin/src/core/ohttp.rs
@@ -60,6 +60,18 @@ pub enum DirectoryResponseError {
     UnexpectedStatusCode(http::StatusCode),
 }
 
+impl DirectoryResponseError {
+    pub(crate) fn is_fatal(&self) -> bool {
+        use DirectoryResponseError::*;
+
+        match self {
+            OhttpDecapsulation(_) => true,
+            InvalidSize(_) => false,
+            UnexpectedStatusCode(status_code) => status_code.is_client_error(),
+        }
+    }
+}
+
 impl fmt::Display for DirectoryResponseError {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         use DirectoryResponseError::*;
diff --git a/payjoin/src/core/persist.rs b/payjoin/src/core/persist.rs
@@ -13,7 +13,6 @@ impl<Event, SuccessValue, CurrentState, Err>
         MaybeSuccessTransitionWithNoResults(Err(Rejection::fatal(event, error)))
     }
 
-    #[allow(dead_code)]
     #[inline]
     pub(crate) fn transient(error: Err) -> Self {
         MaybeSuccessTransitionWithNoResults(Err(Rejection::transient(error)))
@@ -64,6 +63,11 @@ impl<Event, NextState, CurrentState, Err>
         MaybeFatalTransitionWithNoResults(Ok(AcceptOptionalTransition::NoResults(current_state)))
     }
 
+    #[inline]
+    pub(crate) fn transient(error: Err) -> Self {
+        MaybeFatalTransitionWithNoResults(Err(Rejection::transient(error)))
+    }
+
     #[inline]
     pub(crate) fn success(event: Event, next_state: NextState) -> Self {
         MaybeFatalTransitionWithNoResults(Ok(AcceptOptionalTransition::Success(AcceptNextState(
@@ -151,7 +155,7 @@ impl<Event, NextState, Err> MaybeTransientTransition<Event, NextState, Err> {
 /// A transition that can result in the completion of a state machine or a transient error
 /// Fatal errors cannot occur in this transition.
 pub struct MaybeSuccessTransition<Event, SuccessValue, Err>(
-    Result<AcceptNextState<Event, SuccessValue>, RejectTransient<Err>>,
+    Result<AcceptNextState<Event, SuccessValue>, Rejection<Event, Err>>,
 );
 
 impl<Event, SuccessValue, Err> MaybeSuccessTransition<Event, SuccessValue, Err>
@@ -165,7 +169,12 @@ where
 
     #[inline]
     pub(crate) fn transient(error: Err) -> Self {
-        MaybeSuccessTransition(Err(RejectTransient(error)))
+        MaybeSuccessTransition(Err(Rejection::transient(error)))
+    }
+
+    #[inline]
+    pub(crate) fn fatal(event: Event, error: Err) -> Self {
+        MaybeSuccessTransition(Err(Rejection::fatal(event, error)))
     }
 
     pub fn save<P>(
@@ -377,7 +386,10 @@ trait InternalSessionPersister: SessionPersister {
                 self.close().map_err(InternalPersistedError::Storage)?;
                 Ok(success_value)
             }
-            Err(RejectTransient(err)) => Err(InternalPersistedError::Transient(err).into()),
+            Err(Rejection::Transient(RejectTransient(err))) =>
+                Err(InternalPersistedError::Transient(err).into()),
+            Err(Rejection::Fatal(RejectFatal(event, err))) =>
+                Err(self.handle_fatal_reject(RejectFatal(event, err)).into()),
         }
     }
 
@@ -798,6 +810,22 @@ mod tests {
                     MaybeSuccessTransition::transient(InMemoryTestError {}).save(persister)
                 }),
             },
+            // Fatal error
+            TestCase {
+                expected_result: ExpectedResult {
+                    events: vec![InMemoryTestEvent("error event".to_string())],
+                    is_closed: true,
+                    error: Some(InternalPersistedError::Fatal(InMemoryTestError {}).into()),
+                    success: None,
+                },
+                test: Box::new(move |persister| {
+                    MaybeSuccessTransition::fatal(
+                        InMemoryTestEvent("error event".to_string()),
+                        InMemoryTestError {},
+                    )
+                    .save(persister)
+                }),
+            },
         ];
 
         for test in test_cases {
diff --git a/payjoin/src/core/receive/error.rs b/payjoin/src/core/receive/error.rs
@@ -47,7 +47,7 @@ impl error::Error for Error {
     }
 }
 
-/// The protocolerror type for the payjoin receiver, representing failures in
+/// The protocol error type for the payjoin receiver, representing failures in
 /// the internal protocol operation.
 ///
 /// The error handling is designed to:
diff --git a/payjoin/src/core/receive/v2/mod.rs b/payjoin/src/core/receive/v2/mod.rs
@@ -371,16 +371,35 @@ impl Receiver<Initialized> {
         SessionEvent,
         Receiver<UncheckedOriginalPayload>,
         Receiver<Initialized>,
-        Error,
+        ProtocolError,
     > {
         let current_state = self.clone();
         let proposal = match self.inner_process_res(body, context) {
             Ok(proposal) => proposal,
-            Err(e) =>
-                return MaybeFatalTransitionWithNoResults::fatal(
-                    SessionEvent::SessionInvalid(e.to_string(), None),
-                    e,
-                ),
+            Err(e) => match e {
+                // Implementation errors should be unreachable
+                ProtocolError::V2(ref session_error) => match session_error {
+                    SessionError(InternalSessionError::DirectoryResponse(directory_error)) =>
+                        if directory_error.is_fatal() {
+                            return MaybeFatalTransitionWithNoResults::fatal(
+                                SessionEvent::SessionInvalid(e.to_string(), None),
+                                e,
+                            );
+                        } else {
+                            return MaybeFatalTransitionWithNoResults::transient(e);
+                        },
+                    _ =>
+                        return MaybeFatalTransitionWithNoResults::fatal(
+                            SessionEvent::SessionInvalid(session_error.to_string(), None),
+                            e,
+                        ),
+                },
+                _ =>
+                    return MaybeFatalTransitionWithNoResults::fatal(
+                        SessionEvent::SessionInvalid(e.to_string(), None),
+                        e,
+                    ),
+            },
         };
 
         if let Some((proposal, reply_key)) = proposal {
@@ -403,9 +422,9 @@ impl Receiver<Initialized> {
         self,
         body: &[u8],
         context: ohttp::ClientResponse,
-    ) -> Result<Option<(OriginalPayload, Option<HpkePublicKey>)>, Error> {
+    ) -> Result<Option<(OriginalPayload, Option<HpkePublicKey>)>, ProtocolError> {
         let body = match process_get_res(body, context)
-            .map_err(InternalSessionError::DirectoryResponse)?
+            .map_err(|e| ProtocolError::V2(InternalSessionError::DirectoryResponse(e).into()))?
         {
             Some(body) => body,
             None => return Ok(None),
@@ -442,12 +461,13 @@ impl Receiver<Initialized> {
     fn extract_proposal_from_v2(
         self,
         response: Vec<u8>,
-    ) -> Result<(OriginalPayload, HpkePublicKey), Error> {
+    ) -> Result<(OriginalPayload, HpkePublicKey), ProtocolError> {
         let (payload_bytes, reply_key) =
-            decrypt_message_a(&response, self.session_context.receiver_key.secret_key().clone())?;
+            decrypt_message_a(&response, self.session_context.receiver_key.secret_key().clone())
+                .map_err(|e| ProtocolError::V2(InternalSessionError::Hpke(e).into()))?;
         let payload = std::str::from_utf8(&payload_bytes)
             .map_err(|e| ProtocolError::OriginalPayload(InternalPayloadError::Utf8(e).into()))?;
-        self.unchecked_from_payload(payload).map_err(Error::Protocol).map(|p| (p, reply_key))
+        self.unchecked_from_payload(payload).map(|p| (p, reply_key))
     }
 
     fn unchecked_from_payload(self, payload: &str) -> Result<OriginalPayload, ProtocolError> {
@@ -1052,12 +1072,20 @@ impl Receiver<PayjoinProposal> {
         res: &[u8],
         ohttp_context: ohttp::ClientResponse,
     ) -> MaybeSuccessTransition<SessionEvent, (), Error> {
-        match process_post_res(res, ohttp_context)
-            .map_err(|e| InternalSessionError::DirectoryResponse(e).into())
-        {
+        match process_post_res(res, ohttp_context) {
             Ok(_) =>
                 MaybeSuccessTransition::success(SessionEvent::Closed(SessionOutcome::Success), ()),
-            Err(e) => MaybeSuccessTransition::transient(e),
+            Err(e) =>
+                if e.is_fatal() {
+                    MaybeSuccessTransition::fatal(
+                        SessionEvent::SessionInvalid(e.to_string(), None),
+                        InternalSessionError::DirectoryResponse(e).into(),
+                    )
+                } else {
+                    MaybeSuccessTransition::transient(
+                        InternalSessionError::DirectoryResponse(e).into(),
+                    )
+                },
         }
     }
 }
diff --git a/payjoin/src/core/send/v2/mod.rs b/payjoin/src/core/send/v2/mod.rs
@@ -323,12 +323,17 @@ impl Sender<WithReplyKey> {
     ) -> MaybeFatalTransition<SessionEvent, Sender<PollingForProposal>, EncapsulationError> {
         match process_post_res(response, post_ctx.ohttp_ctx) {
             Ok(()) => {}
-            Err(e) => {
-                return MaybeFatalTransition::fatal(
-                    SessionEvent::SessionInvalid(e.to_string()),
-                    InternalEncapsulationError::DirectoryResponse(e).into(),
-                );
-            }
+            Err(e) =>
+                if e.is_fatal() {
+                    return MaybeFatalTransition::fatal(
+                        SessionEvent::SessionInvalid(e.to_string()),
+                        InternalEncapsulationError::DirectoryResponse(e).into(),
+                    );
+                } else {
+                    return MaybeFatalTransition::transient(
+                        InternalEncapsulationError::DirectoryResponse(e).into(),
+                    );
+                },
         }
 
         let polling_for_proposal = PollingForProposal {
@@ -473,10 +478,16 @@ impl Sender<PollingForProposal> {
             Ok(Some(body)) => body,
             Ok(None) => return MaybeSuccessTransitionWithNoResults::no_results(self.clone()),
             Err(e) =>
-                return MaybeSuccessTransitionWithNoResults::fatal(
-                    SessionEvent::SessionInvalid(e.to_string()),
-                    InternalEncapsulationError::DirectoryResponse(e).into(),
-                ),
+                if e.is_fatal() {
+                    return MaybeSuccessTransitionWithNoResults::fatal(
+                        SessionEvent::SessionInvalid(e.to_string()),
+                        InternalEncapsulationError::DirectoryResponse(e).into(),
+                    );
+                } else {
+                    return MaybeSuccessTransitionWithNoResults::transient(
+                        InternalEncapsulationError::DirectoryResponse(e).into(),
+                    );
+                },
         };
         let psbt = match decrypt_message_b(
             &body,

Original file line number	Diff line number	Diff line change
`@@ -47,7 +47,7 @@ impl error::Error for Error {`
`47`	`47`	`}`
`48`	`48`	`}`
`49`	`49`
`50`		`-/// The protocolerror type for the payjoin receiver, representing failures in`
	`50`	`+/// The protocol error type for the payjoin receiver, representing failures in`
`51`	`51`	`/// the internal protocol operation.`
`52`	`52`	`///`
`53`	`53`	`/// The error handling is designed to:`