fix: handle ContractNotFound and TokenNotFound in add token screen#718
fix: handle ContractNotFound and TokenNotFound in add token screen#718thepastaclaw wants to merge 1 commit intodashpay:v1.0-devfrom
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds a one-time fallback from contract-ID to token-ID lookup when a contract lookup returns not-found and the input is valid Base58; introduces Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
✅ Actions performedReview triggered.
|
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/ui/tokens/add_token_by_id_screen.rs`:
- Around line 302-317: The ContractNotFound branch currently always sets
try_token_id_next and causes a loop; add a boolean field tried_token_id_lookup
to the AddTokenByIdScreen state, initialize it false in the constructor and
reset it to false in show_success_screen (and whenever starting a new lookup),
then update display_task_result's BackendTaskSuccessResult::ContractNotFound
handling to only set try_token_id_next = true and set tried_token_id_lookup =
true when tried_token_id_lookup is false (i.e., the initial contract-ID
fallback), and if tried_token_id_lookup is already true treat it as an error
(call MessageBanner::set_global and set self.status = AddTokenStatus::Error) so
a ContractNotFound from the FetchTokenByTokenId path does not loop.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7328d80d-4ac4-4db1-a36d-9046705536c2
📒 Files selected for processing (1)
src/ui/tokens/add_token_by_id_screen.rs
…otFound Add tried_token_id_lookup field to track whether the token ID fallback has already been attempted. Without this, when FetchTokenByTokenId finds a token but its subsequent contract fetch fails (returning ContractNotFound), the handler would set try_token_id_next = true again, creating an infinite loop. The guard ensures ContractNotFound only triggers the token ID fallback once. On subsequent ContractNotFound results, it shows an error banner and transitions to Error state. Addresses CodeRabbit review feedback on dashpay#718.
thepastaclaw
force-pushed
the
fix/token-search-not-found
branch
from
9032752 to
baff28e
Compare
|
Addressed the infinite loop concern — added a @coderabbitai review |
|
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/ui/tokens/add_token_by_id_screen.rs (1)
272-281: Consider adding the same guard todisplay_messagefor consistency.The
display_messagemethod also setstry_token_id_next = truewhen handling "Contract not found" string messages (line 278), but doesn't checktried_token_id_lookup. While the typedBackendTaskSuccessResult::ContractNotFoundnow goes throughdisplay_task_resultcorrectly, this string-based path could theoretically trigger the same infinite loop if messages arrive via this route.♻️ Suggested fix for consistency
} else if msg.contains("Contract not found") { // Contract not found, try as token ID - if let Ok(_identifier) = + if !self.tried_token_id_lookup && Identifier::from_string(&self.contract_or_token_id_input, Encoding::Base58) - Identifier::from_string(&self.contract_or_token_id_input, Encoding::Base58) + .is_ok() { - { // We'll initiate a token ID search self.try_token_id_next = true; + self.tried_token_id_lookup = true; } else { self.status = AddTokenStatus::Error; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/ui/tokens/add_token_by_id_screen.rs` around lines 272 - 281, The display_message handler sets try_token_id_next = true when msg.contains("Contract not found") but lacks the same guard used elsewhere; update display_message (the method handling string messages) to check the tried_token_id_lookup flag before setting try_token_id_next (the same guard used when processing BackendTaskSuccessResult::ContractNotFound) and only set try_token_id_next = true if tried_token_id_lookup is false, otherwise set status to AddTokenStatus::Error to avoid re-triggering the lookup loop; locate display_message and modify the "Contract not found" branch accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/ui/tokens/add_token_by_id_screen.rs`:
- Around line 272-281: The display_message handler sets try_token_id_next = true
when msg.contains("Contract not found") but lacks the same guard used elsewhere;
update display_message (the method handling string messages) to check the
tried_token_id_lookup flag before setting try_token_id_next (the same guard used
when processing BackendTaskSuccessResult::ContractNotFound) and only set
try_token_id_next = true if tried_token_id_lookup is false, otherwise set status
to AddTokenStatus::Error to avoid re-triggering the lookup loop; locate
display_message and modify the "Contract not found" branch accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 82c1ccc3-68af-49a1-b587-1e175bb8c5e1
📒 Files selected for processing (1)
src/ui/tokens/add_token_by_id_screen.rs
thepastaclaw
force-pushed
the
fix/token-search-not-found
branch
from
d8c603d to
3717f62
Compare
|
Also added the @coderabbitai review |
|
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/ui/tokens/add_token_by_id_screen.rs (1)
272-290: Consider extracting the shared fallback logic to reduce duplication.The ContractNotFound handling logic in
display_message(lines 272-290) anddisplay_task_result(lines 315-335) is nearly identical. Extracting this into a private helper method would improve maintainability and ensure both paths stay consistent.♻️ Optional: Extract helper method
+ /// Handles contract-not-found by attempting a one-time fallback to token ID lookup. + fn handle_contract_not_found(&mut self) { + if !self.tried_token_id_lookup + && Identifier::from_string(&self.contract_or_token_id_input, Encoding::Base58).is_ok() + { + self.try_token_id_next = true; + self.tried_token_id_lookup = true; + } else { + MessageBanner::set_global( + self.app_context.egui_ctx(), + "No contract or token found for the given identifier", + MessageType::Error, + ); + self.status = AddTokenStatus::Error; + } + }Then simplify both call sites:
// In display_message(): } else if msg.contains("Contract not found") { self.handle_contract_not_found(); } // In display_task_result(): BackendTaskSuccessResult::ContractNotFound => { self.handle_contract_not_found(); }Also applies to: 315-335
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/ui/tokens/add_token_by_id_screen.rs` around lines 272 - 290, Extract the duplicated "Contract not found" fallback into a private helper on the same impl (e.g., fn handle_contract_not_found(&mut self)) that performs the Identifier::from_string(&self.contract_or_token_id_input, Encoding::Base58).is_ok() check, sets self.try_token_id_next = true and self.tried_token_id_lookup = true when valid, otherwise calls MessageBanner::set_global(self.app_context.egui_ctx(), "No contract or token found for the given identifier", MessageType::Error) and sets self.status = AddTokenStatus::Error; then replace the inline blocks in display_message and the BackendTaskSuccessResult::ContractNotFound arm inside display_task_result with calls to this new handle_contract_not_found method so both code paths share the same logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/ui/tokens/add_token_by_id_screen.rs`:
- Around line 272-290: Extract the duplicated "Contract not found" fallback into
a private helper on the same impl (e.g., fn handle_contract_not_found(&mut
self)) that performs the
Identifier::from_string(&self.contract_or_token_id_input,
Encoding::Base58).is_ok() check, sets self.try_token_id_next = true and
self.tried_token_id_lookup = true when valid, otherwise calls
MessageBanner::set_global(self.app_context.egui_ctx(), "No contract or token
found for the given identifier", MessageType::Error) and sets self.status =
AddTokenStatus::Error; then replace the inline blocks in display_message and the
BackendTaskSuccessResult::ContractNotFound arm inside display_task_result with
calls to this new handle_contract_not_found method so both code paths share the
same logic.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: aeba571b-22c6-4920-9ae4-ca45bd9b2aa5
📒 Files selected for processing (1)
src/ui/tokens/add_token_by_id_screen.rs
The display_task_result method was missing handlers for
BackendTaskSuccessResult::ContractNotFound and
BackendTaskSuccessResult::TokenNotFound variants. These fell through
to the wildcard _ => {} match, causing the screen to stay stuck in
the Searching state indefinitely.
Now:
- ContractNotFound triggers fallback to token ID lookup (existing
try_token_id_next logic), or shows an error banner if the input
is not a valid identifier
- TokenNotFound shows an error banner and transitions to Error state
Fixes dashpay#691
thepastaclaw
force-pushed
the
fix/token-search-not-found
branch
from
3717f62 to
326e1da
Compare
|
Extracted the duplicated |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Code Review
Clean, well-targeted fix for issue #691. The display_task_result wildcard was silently swallowing ContractNotFound and TokenNotFound, leaving the UI stuck in Searching state. The fix correctly handles both variants with a one-time fallback to token-ID lookup and proper Error state transitions. Both agents confirm no issues found.
Reviewed commit: 326e1da
1 participant
Issue
Fixes #691
Description
The Add/Search Token screen gets stuck searching indefinitely when a contract or token is not found on the network.
Root cause:
AddTokenByIdScreen::display_task_resulthas a_ => {}wildcard that silently swallowsBackendTaskSuccessResult::ContractNotFoundandBackendTaskSuccessResult::TokenNotFound. When either variant is returned, the UI stays inSearching(timestamp)state forever with no user feedback.Fix:
ContractNotFound— falls back to interpreting the input as a Token ID (via the existingtry_token_id_nextmechanism), matching the existing fallback pattern elsewhere in the screenTokenNotFound— shows an error viaMessageBannerand transitions toErrorstateMessageBannersystem consistent with other error handling in the same fileValidation
cargo check✅cargo clippy -- -D warnings✅ContractNotFoundandTokenNotFoundare returned by the backend task handlers insrc/backend_task/tokens/mod.rs(lines 523, 549, 553) and were previously swallowed by the wildcardSummary by CodeRabbit