handle failed best_orders RPC exception during login#3263
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughThe PR addresses a race condition during login by adding authentication state guards across multiple bloc and API layers. Changes prevent unauthenticated users from showing loading spinners during balance/max-sell amount updates, implement early unavailable states, short-circuit unnecessary API calls, and add retry logic to best_orders requests. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant UI as UI Layer
participant Bloc as Bloc (bridge/taker/maker)
participant DexRepo as Dex Repository
participant MM2API as MM2 API
Note over User,MM2API: Before Login
User->>UI: Opens bridge/swap form
UI->>Bloc: Request balance/max-sell
Bloc->>Bloc: Check auth status (not logged in)
Bloc-->>UI: Emit unavailable state (no spinner)
Note over User,MM2API: Login occurs
User->>Bloc: Login completes
Note over User,MM2API: After Login
User->>UI: Updates sell coin/amount
UI->>Bloc: _onUpdateMaxSellAmount
Bloc->>Bloc: Check auth status (logged in)
Bloc->>Bloc: Emit loading state
Bloc->>DexRepo: getBestOrders request
DexRepo->>DexRepo: Check menu context
DexRepo->>MM2API: Call with retry wrapper
loop Retry up to 4 times
MM2API->>MM2API: Attempt best_orders call
alt Success
MM2API-->>DexRepo: Return valid result
else Transient error
MM2API->>MM2API: Backoff & retry
end
end
DexRepo-->>Bloc: Return BestOrders
Bloc-->>UI: Emit balance/max-sell state
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
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. Comment |
|
@coderabbitai review @cursor review |
There was a problem hiding this comment.
Pull Request Overview
This PR fixes a race condition that occurred briefly after login where best_orders RPC calls would fail with unhandled errors, causing delays before the app became usable. The changes implement retry logic for best_orders requests and improve the user experience by allowing orderbook viewing before login while showing appropriate balance states.
Key changes:
- Added exponential backoff retry mechanism for best_orders API calls
- Modified balance state handling to show "0.00" instead of loading spinner when not logged in
- Restricted background best_orders calls to only run when user is on swap or bridge pages
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| lib/mm2/mm2_api/mm2_api.dart | Added retry logic with linear backoff for getBestOrders API calls |
| lib/blocs/maker_form_bloc.dart | Updated balance state logic to check login status before showing loading state |
| lib/bloc/taker_form/taker_bloc.dart | Added login status checks to show unavailable balance state instead of loading spinner |
| lib/bloc/dex_repository.dart | Added page-based filtering for best_orders calls and improved error handling |
| lib/bloc/bridge_form/bridge_bloc.dart | Reordered login status check to prevent unnecessary loading states when not logged in |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@CharlVS: I'll review the changes in this pull request for you. ✅ Actions performedReview triggered.
|
|
@cursoragent review |
|
I reviewed the touched files and verified the retry/backoff, pre‑login gating, and page-based filtering for best_orders. Summary
Highlights (with code references)
backoffStrategy: const LinearBackoff(
initialDelay: Duration(milliseconds: 500),
increment: Duration(milliseconds: 250),
maxDelay: Duration(seconds: 3),
),
Future<BestOrders> getBestOrders(BestOrdersRequest request) async {
// Only allow best_orders when user is on Swap (DEX) or Bridge pages
final MainMenuValue current = routingState.selectedMenu;
final bool isTradingPage =
current == MainMenuValue.dex || current == MainMenuValue.bridge;
if (!isTradingPage) {
return BestOrders(result: <String, List<BestOrder>>{});
}
// Before login, show 0.00 instead of spinner
if (!_isLoggedIn) {
emit(
state.copyWith(
availableBalanceState: () => AvailableBalanceState.unavailable,
maxSellAmount: () => null,
),
);
}// If not logged in, show 0.00 (unavailable) and skip spinner
if (!_isLoggedIn) {
emitter(
state.copyWith(
availableBalanceState: () => AvailableBalanceState.unavailable,
maxSellAmount: () => null,
),
);
return;
}
// Only show loading spinner when signed in
final bool isSignedIn = await kdfSdk.auth.isSignedIn();
availableBalanceState =
isSignedIn ? AvailableBalanceState.loading : AvailableBalanceState.unavailable;final Coin? coin = sellCoin;
final bool isSignedIn = await kdfSdk.auth.isSignedIn();
if (!isSignedIn) {
maxSellAmount = null;
availableBalanceState = AvailableBalanceState.unavailable;
return;
}
// Before login, show 0.00 instead of spinner
if (!_isLoggedIn) {
emit(
state.copyWith(
availableBalanceState: () => AvailableBalanceState.unavailable,
maxSellAmount: () => null,
),
);
}// If not logged in, show 0.00 (unavailable) and skip spinner
if (!_isLoggedIn) {
emit(
state.copyWith(
availableBalanceState: () => AvailableBalanceState.unavailable,
maxSellAmount: () => null,
),
);
return;
}Suggestions
backoffStrategy: const LinearBackoff(
initialDelay: Duration(milliseconds: 500),
increment: Duration(milliseconds: 250),
bestOrders.result?.removeWhere(
(coinId, _) => excludedAssetList.contains(coinId),
);bestOrders.result?.removeWhere(
(coinId, _) => excludedAssetList.contains(coinId),
);
Test considerations
Learn more about Cursor Agents |
|
Visit the preview URL for this PR (updated for commit 768fcea): https://walletrc--pull-3263-merge-fcje4b8o.web.app (expires Tue, 11 Nov 2025 18:09:09 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: f66a4ff03faa546f12f0ae5a841bd9eff2714dcc |
|
Cursor Agent can help with this pull request. Just |
![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
♻️ Duplicate comments (1)
lib/mm2/mm2_api/mm2_api.dart (1)
239-259: Refine backoff config and fix spacing.
- Remove the extra space before 500.
- With 4 attempts and 250ms increments, 3s maxDelay is never reached. Either lower maxDelay or increase attempts.
Apply one of these diffs (Option A keeps attempts, Option B keeps maxDelay):
Option A — lower maxDelay:
- backoffStrategy: const LinearBackoff( - initialDelay: Duration(milliseconds: 500), - increment: Duration(milliseconds: 250), - maxDelay: Duration(seconds: 3), - ), + backoffStrategy: const LinearBackoff( + initialDelay: Duration(milliseconds: 500), + increment: Duration(milliseconds: 250), + maxDelay: Duration(milliseconds: 1500), + ),Option B — increase attempts:
- maxAttempts: 4, + maxAttempts: 8, backoffStrategy: const LinearBackoff( - initialDelay: Duration(milliseconds: 500), + initialDelay: Duration(milliseconds: 500), increment: Duration(milliseconds: 250), maxDelay: Duration(seconds: 3), ),
🧹 Nitpick comments (5)
lib/bloc/taker_form/taker_bloc.dart (2)
319-327: Avoid starting the 10s max-sell timer before login.Return early after setting the pre-login state to prevent scheduling periodic updates while logged out.
// Before login, show 0.00 instead of spinner if (!_isLoggedIn) { emit( state.copyWith( availableBalanceState: () => AvailableBalanceState.unavailable, maxSellAmount: () => null, ), ); + return; }
451-461: Remove redundant !_isLoggedIn check later in the method.You already return early for logged-out users above; the check around Lines 490-496 is unreachable and can be removed.
- if (!_isLoggedIn) { - emitter( - state.copyWith( - availableBalanceState: () => AvailableBalanceState.unavailable, - ), - ); - } else { + { Rational? maxSellAmount = await _dexRepo.getMaxTakerVolume( state.sellCoin!.abbr, ); if (maxSellAmount != null) { emitter( state.copyWith( maxSellAmount: () => maxSellAmount, availableBalanceState: () => AvailableBalanceState.success, ), ); } else { maxSellAmount = await _frequentlyGetMaxTakerVolume(); emitter( state.copyWith( maxSellAmount: () => maxSellAmount, availableBalanceState: maxSellAmount == null ? () => AvailableBalanceState.failure : () => AvailableBalanceState.success, ), ); } }lib/bloc/dex_repository.dart (2)
22-24: Decouple repository from routing/UI state.Importing and reading routingState from the repository creates an unnecessary UI dependency. Prefer pushing this guard up to the blocs or inject a predicate (e.g., bool Function() isTradingContext) via constructor.
Also applies to: 127-134
139-146: Minor: adjust log path for clarity.These logs originate in the repository; consider using “repo => getBestOrders” instead of “api => …” to ease triage.
- path: 'api => getBestOrders', + path: 'repo => getBestOrders',Apply similarly in both log calls.
Also applies to: 154-160
lib/bloc/bridge_form/bridge_bloc.dart (1)
241-249: Skip scheduling the periodic balance poll before login.After setting the pre-login state, return early to avoid starting _subscribeMaxSellAmount while logged out.
// Before login, show 0.00 instead of spinner if (!_isLoggedIn) { emit( state.copyWith( availableBalanceState: () => AvailableBalanceState.unavailable, maxSellAmount: () => null, ), ); + return; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
lib/bloc/bridge_form/bridge_bloc.dart(2 hunks)lib/bloc/dex_repository.dart(3 hunks)lib/bloc/taker_form/taker_bloc.dart(2 hunks)lib/blocs/maker_form_bloc.dart(2 hunks)lib/mm2/mm2_api/mm2_api.dart(1 hunks)
⏰ 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). (7)
- GitHub Check: Deploy Preview
- GitHub Check: validate_code_guidelines
- GitHub Check: Build Desktop (linux)
- GitHub Check: Build Desktop (windows)
- GitHub Check: build_and_preview
- GitHub Check: Build Mobile (Android)
- GitHub Check: unit_tests
🔇 Additional comments (2)
lib/bloc/bridge_form/bridge_bloc.dart (2)
405-414: LGTM: early return when not logged in.Prevents spinners and unnecessary calls while logged out.
416-424: LGTM: gate loading state by auth or explicit setLoadingStatus.Keeps UX consistent and avoids misleading loading indicators.
smk762
added
bug
smk762
changed the title
best_orders exception during login
smk762
changed the title
best_orders exception during loginbest_orders RPC exception during login
takenagain
left a comment
takenagain
left a comment
There was a problem hiding this comment.
Confirmed that the error logs are no longer present in web logs and that test flow passes
…ty (#3328) - Propagate errors from best_orders instead of returning empty results - Treat P2P 'No response from any peer' (including wrapped Exception) as no orders - Add debug-only failure simulator toggled via kSimulateBestOrdersFailure feat(ui): CTA to Maker when no orders; contextual localized messages - Taker: show CTA when no swap offers available - Bridge: show CTA when no cross-network routes exist - Add locale keys: dexNoSwapOffers, bridgeNoCrossNetworkRoutes
Closes #3261
This issue was limited to a brief period post login, where the rpc would return an unhandled error, leading to delays between login and app being usable.
This PR:
To test:
0(no spinner)Repeat for bridge page.
Summary by CodeRabbit
Note
Retries and hardens
best_orders, limits background calls to trading pages, defers balance polling until after login, and updates UI/messages to show clear no-liquidity CTAs.Mm2Api.getBestOrders: add retry with linear backoff; return error text on exception.DexRepository.getBestOrders: only fetch ondex/bridgepages; map "no response from any peer" to empty results; add logging; optional simulated failures viakSimulateBestOrdersFailure.TakerBloc,BridgeBloc,MakerFormBloc: showAvailableBalanceState.unavailable(0.00) before login; skip spinners and balance polling; reinitialize balances immediately upon login or page entry._NoOrdersCtawith Maker switch; improved error message with retry.dexNoSwapOffers,bridgeNoCrossNetworkRoutestoen.jsonandcodegen_loader.best_ordersfailures.Written by Cursor Bugbot for commit f52a907. This will update automatically on new commits. Configure here.