Swallowed errors in process_own_* self-publish reaction handlers (mls_sync.rs)

[insipx]

Summary

On the self-sent / optimistic-publish path (process_own_message in crates/xmtp_mls/src/groups/mls_sync.rs), two follow-up reaction handlers discard their errors instead of propagating, which can silently drop retryable failures.

• process_own_leave_request_message returns () and matches the result of process_leave_request_message, dropping Err to a debug!:

  match self.process_leave_request_message(mls_group, storage, &message, None) {
      Ok(()) => debug!("Successfully processed leave request message"),
      Err(e) => debug!("Failed to process leave request message: {}", e),
  }

• process_own_delete_message returns () and uses let Ok(Some(..)) = db.get_group_message(..) else { return } (and similar), so DB read failures vanish silently.

Both are called from process_own_message, which itself returns a Result — so propagation is available; the swallow is a deliberate-but-undocumented choice.

Why it's a problem

GroupMessageProcessingError already implements is_retryable(). The current code flattens every failure into a silent no-op, including transient/retryable storage errors that elsewhere in process_message_inner cause the sync transaction to roll back and retry. So a retryable failure on a client's own leave/delete reaction is dropped, and even a hard failure is only visible at debug!.

This is the classic silent-failure smell: best-effort intent is fine for genuinely benign cases, but it must (a) not swallow retryable errors, and (b) surface real failures above debug!.

Proposed fix

1. Return Result<(), GroupMessageProcessingError> from both handlers; replace silent let Ok(..) else { return } reads with ?.
2. In process_own_message, branch on err.is_retryable():
   • retryable → propagate (roll back + retry the sync), matching the rest of the message-processing path;
   • non-retryable → log at warn!/error! with message/group context and continue (the message was already published; the local reaction is genuinely best-effort here), with a comment documenting the intent.
3. Keep the content_type guards (correct early-returns, not errors).

Testing

• Existing test_self_removal* + xmtpv3 binding tests + delete-message tests must still pass.
• Add a test injecting a retryable storage error into the own-leave/own-delete reaction, asserting the sync surfaces it (rolls back / reports in summary) rather than silently succeeding.

Notes

• Pre-existing behavior (predates the event-driven self-remove work in feat(xmtp_mls): event-driven pending-self-remove via TaskRunner #3744); should be addressed in its own PR.
• Surfaced while reviewing feat(xmtp_mls): event-driven pending-self-remove via TaskRunner #3744.

🤖 Generated with Claude Code

[Jr-kenny]

Hey, please could you assign this one to me? I'd like to work on it.