refactor: migrate remaining flat/forwarding RetryableError impls to derive (tier 1)

[insipx]

Summary

Stacked on #3754 (which stacks on #3753) — merge in order.

Completes the #[derive(Retryable)] migration for every remaining impl that the macro can express, including the heavyweights deliberately deferred from #3754: GroupError (45 variants), SubscribeError, StorageError, ConnectionError, ApiClientError, GroupMessageProcessingError. 17 hand-written impls deleted across 6 crates + xnet.

What each shape became

• ~80 forwarding arms → #[retry(inherit)]
• Hardcoded trues (GroupError::LockUnavailable, SubscribeError::StreamStale, EnvelopeError::Decode, ApiClientError::Http/Expired, …) → #[retry(true)]
• All-true enums (GrpcError, wasm PlatformStorageError) → #[retry(default = true)]; all-false validation enums → bare derive, zero attrs
• Multi-field forwards (ApiClientError::ClientWithEndpoint, BootstrapSynthesisError::IdentityUpdateLookup, CommitLogError::FailedToSendReadd) → #[retry(when = source.is_retryable())]
• Collection aggregation (CommitLogError::FailedReadds) → #[retry(when = errors.iter().any(|e| e.is_retryable()))]
• GroupMessageProcessingError's nested match collapsed to inherit by deriving on the inner ProcessMessageWithAppDataError (the one API-surface change: that generic enum gains a ProcessMessageError<S>: RetryableError<Mls> where-bound, per the derive's documented generics rule; the only workspace instantiation already satisfies it)

Still hand-written, by design

The 22 specialized impl RetryableError<Mls> for openmls::*/diesel orphan-rule impls (mirror-type macro support would cost more code than it removes), GroupAppDataError<SqlKeyStoreError> (instantiation-specific), ReceiveErrors/SyncSummary one-line aggregators, and test mocks. After this PR, hand-written impls are a single explainable category: foreign-type orphan workarounds.

Verification

• Ground truth captured verbatim before any edit; adversarial audit re-derived all 204 variants against it: zero mismatches
• 32 new golden tests pin pre-migration booleans (every forwarding direction, every hardcoded-true oddball, the catch-all-false pins); 73 retryable tests green across touched crates
• cargo check --workspace --all-targets clean; wasm cargo check -p xmtp_db --target wasm32-unknown-unknown clean (wasm PlatformStorageError migrated); fmt + clippy -Dwarnings clean

🤖 Generated with Claude Code

Note

Migrate manual RetryableError implementations to #[derive(Retryable)] across tier-1 crates

Replaces hand-written impl RetryableError blocks across ~18 files with #[derive(Retryable)] and per-variant #[retry(...)] annotations from xmtp_common.

• Each variant is annotated with #[retry(true)], #[retry(inherit)], or a conditional expression (e.g. #[retry(when = source.is_retryable())]) to preserve existing retryability semantics exactly.
• Enums where all variants were unconditionally retryable use #[retry(default = true)] at the enum level, and enums where all variants were non-retryable are left unannotated.
• Golden tests are added alongside most conversions to pin retryability behavior and catch future regressions.
• Affected crates include xmtp_api_d14n, xmtp_api_grpc, xmtp_db, xmtp_mls, xmtp_proto, and xnet.
• Behavioral Change: retryability logic is unchanged in intent, but is now enforced at compile time via the derive macro rather than runtime hand-written logic.

^{Macroscope summarized 2455683.}

[claude]

Claude finished @insipx's task —— View job

---

Code Review Complete

Review Date: 2026-06-10 20:02 UTC

Overview

This PR successfully completes the migration of the remaining hand-written RetryableError implementations to the #[derive(Retryable)] macro, including major enums like GroupError (45 variants), SubscribeError, StorageError, CommitLogError, and ApiClientError. The migration is comprehensive, well-tested, and maintains backward compatibility.

✅ Strengths

1. Excellent test coverage - 32 new golden tests pin pre-migration behavior across all touched crates
2. Comprehensive documentation - Each variant has clear doc comments explaining retryability
3. Proper attribute usage - Correct application of #[retry(inherit)], #[retry(true)], #[retry(when = ...)], and #[retry(default = true)]
4. Verification rigor - Author's adversarial audit of all 204 variants with zero mismatches
5. Clean migration patterns:
   • ~80 forwarding arms → #[retry(inherit)]
   • Hardcoded true → #[retry(true)]
   • All-true/all-false enums → #[retry(default = true)] or bare derive
   • Multi-field forwards → #[retry(when = source.is_retryable())]
   • Collection aggregation → #[retry(when = errors.iter().any(|e| e.is_retryable()))]

Findings

No critical issues found. The migration is high-quality with excellent attention to detail.

Minor observations (not blocking):

1. crates/xmtp_api_d14n/src/queries/mod.rs:78 - QueryError::Envelope doesn't inherit retryability even though the inner EnvelopeError can be retryable (golden test confirms this was intentional pre-migration behavior)

2. Complex #[retry(when = ...)] expressions - Some are quite sophisticated (e.g., CommitLogError::FailedReadds checking errors.iter().any(|e| e.is_retryable())). These look correct but could be more maintainable with helper methods if this pattern repeats elsewhere.

3. Nested error forwarding - The needs_db_reconnect() implementations correctly follow nested errors (e.g., CommitLogError::FailedToSendReadd forwards to source, FailedReadds checks all errors). Good defensive depth.

Code Quality Assessment

• ✅ Correctness: Golden tests validate all migrations match original behavior
• ✅ Best practices: Consistent attribute usage, comprehensive documentation
• ✅ Performance: No concerns - macro generates equivalent code
• ✅ Security: No concerns identified
• ✅ Test coverage: Excellent - 32 golden tests + 73 retryable tests passing

Conclusion

This is a well-executed refactoring that:

• Reduces boilerplate (17 hand-written impls deleted)
• Improves maintainability through macro consistency
• Maintains 100% behavioral compatibility
• Adds comprehensive test coverage

Recommendation: Approve and merge. The PR is ready to land after #3754.

---

[macroscopeapp]

Approvability

Verdict: Approved

Mechanical refactoring that migrates manual RetryableError trait implementations to #[derive(Retryable)] macros. Behavior is preserved and verified by extensive golden tests. All files are owned by the author.

^{You can customize Macroscope's approvability policy. Learn more.}

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 84.91%. Comparing base (526c596) to head (2455683).

Additional details and impacted files

@@                      Coverage Diff                       @@
##           insipx/retryable-migration    #3757      +/-   ##
==============================================================
+ Coverage                       84.63%   84.91%   +0.27%     
==============================================================
  Files                             407      407              
  Lines                           59818    59871      +53     
==============================================================
+ Hits                            50629    50840     +211     
+ Misses                           9189     9031     -158     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[insipx]

Folded into #3754 per review simplification — one migration PR instead of two, and per-enum golden tests dropped in favor of the macro's own test suite as the behavioral contract (net −456 lines). The tier-1 commit content is now part of #3754.