feat: add orphan session cleanup for create orders with 10s timeout #318
Conversation
- Introduce startSessionTimeoutCleanupForRequestId() to handle orphan sessions when creating a new order without a response from Mostro within 10s - Add cancelSessionTimeoutCleanupForRequestId() to stop timers on any response to avoid false cleanups - Integrate cleanup logic in AddOrderNotifier: - Start timeout after submitting a new order - Cancel timeout when receiving response or disposing notifier - Extend SessionNotifier with deleteSessionByRequestId() for in-memory session cleanup - Increase action button timeout from 5s to 10s for consistency with cleanup window - Move timer cancellation from handleEvent() to subscribe() in AbstractMostroNotifier so cancellation applies reliably to all responses
WalkthroughAdds 10s request-scoped session timeout management alongside existing per-order timeouts: new start/cancel APIs for requestId, exposed per-order cancellation, integration into AddOrderNotifier lifecycle (start on submit, cancel on response/dispose), SessionNotifier gains deleteSessionByRequestId, and UI timeout increased to 10s. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant UI as ActionButtons
participant Add as AddOrderNotifier
participant Abs as AbstractMostroNotifier
participant Sess as SessionNotifier
participant Mostro as Mostro Service
User->>UI: Tap Submit
UI->>Add: submitOrder(requestId)
Add->>Abs: startSessionTimeoutCleanupForRequestId(requestId, ref) [start 10s timer]
Add->>Mostro: publish/submitOrder(requestId)
Note over Abs,Sess: If 10s elapse with no response
Abs-->>Sess: deleteSessionByRequestId(requestId)
Abs-->>UI: show timeout notification / navigate home
Abs--x Mostro: (no response)
Mostro-->>Add: Response (success or CantDo)
Add->>Abs: cancelSessionTimeoutCleanupForRequestId(requestId)
alt Response contains orderId
Abs->>Abs: cancelSessionTimeoutCleanup(orderId) (per-order timer)
end
Add-->>UI: update state (confirmed or error handling)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (4)
🧰 Additional context used📓 Path-based instructions (5)docs/**📄 CodeRabbit inference engine (AGENTS.md)
Files:
lib/**/*.dart📄 CodeRabbit inference engine (CLAUDE.md)
Files:
lib/features/**📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.dart📄 CodeRabbit inference engine (AGENTS.md)
Files:
lib/features/**/*.dart📄 CodeRabbit inference engine (AGENTS.md)
Files:
🧠 Learnings (7)📚 Learning: 2025-06-26T15:03:23.529ZApplied to files:
📚 Learning: 2025-09-22T18:48:23.369ZApplied to files:
📚 Learning: 2025-09-22T18:48:23.369ZApplied to files:
📚 Learning: 2025-07-20T23:33:17.689ZApplied to files:
📚 Learning: 2025-05-06T15:49:55.079ZApplied to files:
📚 Learning: 2025-09-22T18:48:23.369ZApplied to files:
📚 Learning: 2025-09-22T18:48:23.369ZApplied to files:
🔇 Additional comments (10)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Tip 🧪 Early access (models): enabledWe are currently testing Sonnet 4.5 code review models, which should lead to better review quality. However, this model may result in higher noise levels in the review comments. Please disable the early access features if the noise level causes any inconvenience. Note:
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
lib/features/order/notfiers/abstract_mostro_notifier.dart(2 hunks)lib/features/order/notfiers/add_order_notifier.dart(4 hunks)lib/features/order/widgets/action_buttons.dart(1 hunks)lib/shared/notifiers/session_notifier.dart(1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
lib/**/*.dart
📄 CodeRabbit inference engine (CLAUDE.md)
lib/**/*.dart: Always use localized strings viaS.of(context)!.keyName; avoid hardcoded user-facing text
Useconstconstructors where possible for immutability and performance
Checkmountedbefore usingBuildContextafter async operations
UseS.of(context)!.keyNameand passBuildContextto methods needing localization
Files:
lib/shared/notifiers/session_notifier.dartlib/features/order/notfiers/add_order_notifier.dartlib/features/order/widgets/action_buttons.dartlib/features/order/notfiers/abstract_mostro_notifier.dart
lib/shared/**
📄 CodeRabbit inference engine (AGENTS.md)
Put shared utilities under lib/shared/
Files:
lib/shared/notifiers/session_notifier.dart
**/*.dart
📄 CodeRabbit inference engine (AGENTS.md)
**/*.dart: Format code with the Dart formatter (two-space indentation, trailing commas where beneficial)
Keep the analyzer (flutter_lints) at zero warnings across the codebase
Files:
lib/shared/notifiers/session_notifier.dartlib/features/order/notfiers/add_order_notifier.dartlib/features/order/widgets/action_buttons.dartlib/features/order/notfiers/abstract_mostro_notifier.dart
lib/features/**
📄 CodeRabbit inference engine (AGENTS.md)
Place feature-specific Dart code under lib/features// (screens, providers, widgets)
Files:
lib/features/order/notfiers/add_order_notifier.dartlib/features/order/widgets/action_buttons.dartlib/features/order/notfiers/abstract_mostro_notifier.dart
lib/features/**/*.dart
📄 CodeRabbit inference engine (AGENTS.md)
Name Riverpod providers as Provider or Notifier within their feature directory
Files:
lib/features/order/notfiers/add_order_notifier.dartlib/features/order/widgets/action_buttons.dartlib/features/order/notfiers/abstract_mostro_notifier.dart
lib/features/**/{screens,providers,notifiers,widgets}/**/*.dart
📄 CodeRabbit inference engine (CLAUDE.md)
Follow feature-based organization:
features/{feature}/{screens|providers|notifiers|widgets}/
Files:
lib/features/order/widgets/action_buttons.dart
🧠 Learnings (4)
📚 Learning: 2025-09-22T18:48:23.369Z
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-22T18:48:23.369Z
Learning: Applies to lib/features/**/notifiers/**/*.dart : Use Notifier pattern for complex state logic (authentication, order management) and place in `features/{feature}/notifiers/`
Applied to files:
lib/features/order/notfiers/add_order_notifier.dartlib/features/order/notfiers/abstract_mostro_notifier.dart
📚 Learning: 2025-06-26T15:03:23.529Z
Learnt from: chebizarro
PR: MostroP2P/mobile#127
File: lib/features/order/notfiers/abstract_mostro_notifier.dart:45-54
Timestamp: 2025-06-26T15:03:23.529Z
Learning: In AbstractMostroNotifier, state updates occur for all messages regardless of timestamp to hydrate the OrderNotifier from MostroStorage during sync, while handleEvent is only called for recent messages (within 60 seconds) to prevent re-triggering side effects like notifications and navigation for previously handled messages. This design prevents displaying stale notifications when the app is reopened or brought to the foreground.
Applied to files:
lib/features/order/notfiers/add_order_notifier.dartlib/features/order/notfiers/abstract_mostro_notifier.dart
📚 Learning: 2025-06-04T19:35:20.209Z
Learnt from: chebizarro
PR: MostroP2P/mobile#110
File: test/notifiers/take_order_notifier_test.dart:72-74
Timestamp: 2025-06-04T19:35:20.209Z
Learning: MostroService methods like takeBuyOrder() and takeSellOrder() return Future<void> and trigger side effects through other mechanisms rather than direct return values. When testing these methods, focus on verifying method calls and testing state changes through the provider system rather than mocking return values.
Applied to files:
lib/features/order/notfiers/add_order_notifier.dart
📚 Learning: 2025-09-22T18:48:23.369Z
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-22T18:48:23.369Z
Learning: Applies to lib/features/relays/relays_notifier.dart : Implement relay sync and blacklist operations in `RelaysNotifier` (`syncWithMostroInstance`, `removeRelayWithBlacklist`, `addRelayWithSmartValidation`)
Applied to files:
lib/features/order/notfiers/abstract_mostro_notifier.dart
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (6)
lib/shared/notifiers/session_notifier.dart (1)
190-196: Fix return type mismatch indeleteSessionByRequestIdDeclaring the method as
Future<void>without marking itasync(or explicitly returning aFuture) produces a compile-time error: the function ends without returning aFuture, so the analyzer reports “This function has a return type of ‘Future’ but doesn’t end with a return statement.” Please addasync(or returnFuture.value()) so callers awaiting this method continue to compile.- Future<void> deleteSessionByRequestId(int requestId) async { + Future<void> deleteSessionByRequestId(int requestId) async { _requestIdToSession.remove(requestId); // Note: No storage deletion - these are temporary sessions in memory only _emitState(); }Likely an incorrect or invalid review comment.
lib/features/order/notfiers/add_order_notifier.dart (4)
43-45: LGTM: Cancelling on CantDo avoids stray cleanups.Clearing the per-request timeout as soon as any CantDo arrives keeps the orphan-session cleanup from firing after we already handled the response.
64-66: LGTM: Success path clears timeout.Dropping the timer as soon as we confirm the order ensures no lingering cleanup will race with the success flow.
90-92: LGTM: Starting the 10 s watchdog on submit.Kicking off the per-request timer right after opening the session matches the new orphan cleanup contract.
117-120: LGTM: Dispose path tidies timers.Canceling the outstanding request timer during dispose keeps background cleanups from firing after this notifier is gone.
lib/features/order/widgets/action_buttons.dart (1)
52-52: LGTM: UI timeout matches backend window.Extending the reactive button timeout to 10 s keeps the UX aligned with the new session cleanup horizon.
fix #277
Summary by CodeRabbit
Improvements
Bug Fixes
Documentation