fix: return error on oracle fetch failure instead of silently continuing execution#2532
Open
omaidf wants to merge 1 commit intorainlanguage:mainfrom
Open
fix: return error on oracle fetch failure instead of silently continuing execution#2532omaidf wants to merge 1 commit intorainlanguage:mainfrom
omaidf wants to merge 1 commit intorainlanguage:mainfrom
Conversation
Previously, if fetch_signed_context failed, the error was logged as a warning and execution proceeded with empty signed_context. This is inconsistent with the quoting and candidate-building paths, which both treat oracle failure as a hard error. Orders should not execute without oracle price data. Ref: rainlanguage#2531
Contributor
📝 WalkthroughWalkthroughThe change modifies error handling in the Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
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 `@crates/common/src/raindex_client/take_orders/single.rs`:
- Around line 159-166: The match in execute_single_take that calls
crate::oracle::fetch_signed_context returns an OracleError which cannot be
converted via e.into() to the function's RaindexError return type; replace the
e.into() usage by returning the appropriate RaindexError variant or add a
conversion impl. Concretely, either implement From<OracleError> for
RaindexError, or change the Err(e) arm in the fetch_signed_context match to
return Err(RaindexError::OracleFetchError(e.to_string())) while keeping the
tracing::error log and assignment to candidate.signed_context on success.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 953ee674-b800-4b56-9ec6-d124110e9cfd
📒 Files selected for processing (1)
crates/common/src/raindex_client/take_orders/single.rs
Comment on lines
159
to
166
| match crate::oracle::fetch_signed_context(&url, body).await { | ||
| Ok(ctx) => candidate.signed_context = vec![ctx], | ||
| Err(e) => { | ||
| tracing::warn!("Failed to fetch oracle data: {}", e); | ||
| tracing::error!("Oracle fetch failed, skipping order execution: {}", e); | ||
| return Err(e.into()); | ||
| } | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for Into<RaindexError> or From implementations for oracle errors
rg -n "impl.*From.*for RaindexError" -A 3 | head -50
# Check the oracle module's error types
ast-grep --pattern 'impl From<$_> for RaindexError {
$$$
}'Repository: rainlanguage/raindex
Length of output: 2143
🏁 Script executed:
#!/bin/bash
# Find oracle module and its error definition
find crates -name "*.rs" -type f | xargs grep -l "mod oracle\|pub mod oracle" | head -5
# Search for the oracle module location
fd oracle crates --type fRepository: rainlanguage/raindex
Length of output: 167
🏁 Script executed:
#!/bin/bash
# Search for fetch_signed_context function
rg -n "fn fetch_signed_context" -A 10 crates/
# Check for OracleError type definition
rg -n "enum OracleError\|struct OracleError" -A 5 crates/Repository: rainlanguage/raindex
Length of output: 1526
🏁 Script executed:
#!/bin/bash
# Get the full execute_single_take function signature
rg -n "async fn execute_single_take" crates/common/src/raindex_client/take_orders/single.rs -A 5
# Search for From impl for OracleError
rg -n "impl.*From.*OracleError" crates/
# Check RaindexError enum definition
rg -n "enum RaindexError" crates/common/src/raindex_client/mod.rs -A 20Repository: rainlanguage/raindex
Length of output: 1236
🏁 Script executed:
#!/bin/bash
# Search for all From implementations related to OracleError
rg -n "From<OracleError>" crates/
# Check the oracle module error definition
cat crates/quote/src/oracle.rs | head -100
# Search in common crate's oracle.rs if it exists
if [ -f crates/common/src/oracle.rs ]; then
cat crates/common/src/oracle.rs | head -100
fiRepository: rainlanguage/raindex
Length of output: 3563
🏁 Script executed:
#!/bin/bash
# Search explicitly for From<OracleError> for RaindexError
rg -n "From<OracleError> for RaindexError" crates/
# Check if there's any conversion pattern in the actual file being reviewed
sed -n '150,170p' crates/common/src/raindex_client/take_orders/single.rs
# Get context around the problematic line
sed -n '159,166p' crates/common/src/raindex_client/take_orders/single.rsRepository: rainlanguage/raindex
Length of output: 1174
🏁 Script executed:
#!/bin/bash
# Check more thoroughly for From impl for OracleError
rg -B2 -A10 "impl.*From.*OracleError" crates/
# Check if OracleError has a Display/ToString impl that might be used
rg -n "impl.*Display.*OracleError\|impl.*ToString.*OracleError" crates/
# Look at the entire mod.rs to see all From impls
rg -n "^impl From" crates/common/src/raindex_client/mod.rs
# Check if there's a custom conversion in the file being reviewed
rg -n "OracleError" crates/common/src/raindex_client/take_orders/single.rsRepository: rainlanguage/raindex
Length of output: 391
🏁 Script executed:
#!/bin/bash
# Search for any usage of OracleFetchError or OracleError in the codebase
rg -n "OracleFetchError\|OracleError" crates/common/src/raindex_client/take_orders/single.rs
# Check if there's a recent cargo build or test that would catch this
cd crates/common && cargo check --message-format=short 2>&1 | head -100Repository: rainlanguage/raindex
Length of output: 310
🏁 Script executed:
#!/bin/bash
# Check the exact signature of fetch_signed_context in common crate
rg -n "pub use.*oracle\|pub async fn fetch_signed_context" crates/common/src/oracle.rs
# Look at the imports in single.rs
head -30 crates/common/src/raindex_client/take_orders/single.rs
# Check if oracle.rs in common crate has its own fetch_signed_context
cat crates/common/src/oracle.rsRepository: rainlanguage/raindex
Length of output: 1359
🏁 Script executed:
#!/bin/bash
# Verify the exact type that fetch_signed_context returns in the error case
rg -B5 -A15 "pub async fn fetch_signed_context" crates/quote/src/oracle.rs | head -30
# Check what OracleError is exactly
rg -B2 -A8 "pub enum OracleError" crates/quote/src/oracle.rs
# Now check if there's any From impl for OracleError anywhere
rg "impl From.*for.*RaindexError" crates/common/ -A 5 | grep -A 5 OracleRepository: rainlanguage/raindex
Length of output: 1667
🏁 Script executed:
#!/bin/bash
# Look at the full RaindexError enum definition with all variants
rg -n "enum RaindexError" crates/common/src/raindex_client/mod.rs -A 40
# Check if there's a From<OracleError> anywhere in the entire codebase
rg "From<OracleError>" --type rs
# Check if there's a blanket impl or similar
rg "impl.*Into.*RaindexError\|impl.*From.*OracleError" crates/ --type rsRepository: rainlanguage/raindex
Length of output: 1832
🏁 Script executed:
#!/bin/bash
# Search for any From<OracleError> without the --type flag
rg "From<OracleError>"
# Search for all impl From blocks and look for oracle
rg -n "impl From" crates/common/src/raindex_client/mod.rs
# Check the Display impl for OracleError which has thiserror
rg -B5 -A10 "OracleError" crates/quote/src/oracle.rs | grep -A 10 "enum OracleError"
# Look for any automatic conversions or helper functions
rg -n "\.into\(\)" crates/common/src/raindex_client/take_orders/single.rs | head -20Repository: rainlanguage/raindex
Length of output: 683
🏁 Script executed:
#!/bin/bash
# Get the complete context around line 163 with more lines
sed -n '155,175p' crates/common/src/raindex_client/take_orders/single.rs
# Check if there's a From impl for OracleError anywhere that might use .to_string() or similar
rg -C 10 "OracleFetchError" crates/common/src/raindex_client/mod.rsRepository: rainlanguage/raindex
Length of output: 2153
🏁 Script executed:
#!/bin/bash
# Search more thoroughly for any From impl related to oracle across entire codebase
find crates -name "*.rs" -exec grep -l "OracleError\|OracleFetchError" {} \;
# Check if there's an impl in quote/lib.rs or quote/mod.rs
grep -r "From<OracleError>" crates/quote/ 2>/dev/null || echo "Not found in quote"
# Look for the impl in common more carefully
grep -r "impl From<OracleError>" crates/common/ 2>/dev/null || echo "Not found in common"
# Try to find any conversions
grep -rn "OracleError" crates/common/src/raindex_client/mod.rsRepository: rainlanguage/raindex
Length of output: 213
🏁 Script executed:
#!/bin/bash
# Check candidates.rs to see how it uses OracleError
grep -B5 -A5 "OracleError" crates/common/src/take_orders/candidates.rs
# Check if there are any other error handling patterns in single.rs
rg -n "\.into()" crates/common/src/raindex_client/take_orders/single.rs -B 2 -A 2
# Check the git history to see if this is a recent change
cd crates/common && git log --oneline -n 20 src/raindex_client/take_orders/single.rs 2>/dev/null || echo "Git not available"Repository: rainlanguage/raindex
Length of output: 341
🏁 Script executed:
#!/bin/bash
# Check the git diff for this commit to see the exact change
cd crates/common && git show d4c2e9ab4 --stat 2>/dev/null || echo "Cannot show git diff"
# Try a different approach - check if there's a generic From impl or trait impl
rg -n "impl.*From.*OracleError\|impl.*From.*<.*>" crates/common/src/raindex_client/mod.rs -A 3
# Check if OracleError implements Into or has a Display that could be used
rg -B3 -A8 "thiserror::Error" crates/quote/src/oracle.rs | head -40
# Critical: Try to find if there's a newtype wrapper or intermediate type
rg -n "OracleFetchError" crates/ -B 2 -A 2Repository: rainlanguage/raindex
Length of output: 50588
🏁 Script executed:
#!/bin/bash
# Check if the code actually compiles by looking for any workarounds or conversion patterns
rg -n "map_err\|\.into()\|From<OracleError>" crates/common/src/raindex_client/take_orders/single.rs -B 2 -A 2
# Check if the recent commit might have added a From impl elsewhere
git log --all --oneline -n 50 | head -20
# Try to see just the actual changed line in that commit
cd crates/common && git show d4c2e9ab4:src/raindex_client/take_orders/single.rs 2>/dev/null | sed -n '150,170p' || echo "Cannot retrieve file from commit"Repository: rainlanguage/raindex
Length of output: 1557
🏁 Script executed:
#!/bin/bash
# Look for all From implementations in the entire codebase more broadly
find crates -name "*.rs" -type f -exec grep -l "impl From" {} \; | head -10
# Check if there's a From impl for OracleError that converts to a string or wrapper
grep -r "From<OracleError>" crates/ 2>/dev/null
# Check the exact location of the OracleError definition and any From impls near it
sed -n '1,100p' crates/quote/src/oracle.rs | grep -A 20 "enum OracleError"
# Most importantly: try to find if there's a From<OracleError> impl anywhere using a broader search
rg "impl.*From" crates/ | grep -i oracleRepository: rainlanguage/raindex
Length of output: 2090
The code cannot compile as written. The e.into() call on line 163 will fail because there is no From<OracleError> for RaindexError implementation in the codebase.
The execute_single_take function returns Result<TakeOrdersCalldataResult, RaindexError>, and the Err(e) must be converted to RaindexError, but no such conversion exists. The RaindexError::OracleFetchError(String) variant lacks the #[from] attribute and no manual impl is defined.
Fix: Either add a From<OracleError> for RaindexError implementation, or change the error handling to:
Err(e) => {
tracing::error!("Oracle fetch failed, skipping order execution: {}", e);
return Err(RaindexError::OracleFetchError(e.to_string()));
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/common/src/raindex_client/take_orders/single.rs` around lines 159 -
166, The match in execute_single_take that calls
crate::oracle::fetch_signed_context returns an OracleError which cannot be
converted via e.into() to the function's RaindexError return type; replace the
e.into() usage by returning the appropriate RaindexError variant or add a
conversion impl. Concretely, either implement From<OracleError> for
RaindexError, or change the Err(e) arm in the fetch_signed_context match to
return Err(RaindexError::OracleFetchError(e.to_string())) while keeping the
tracing::error log and assignment to candidate.signed_context on success.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Problem
In
single.rs:159-164, whenfetch_signed_contextfails, the error is logged as awarn!and execution continues with emptysigned_context. The order gets submitted to the on-chain orderbook without oracle price data.This is inconsistent with how the same failure is handled elsewhere:
order_quotes.rs): returns error response, quote skippedcandidates.rs): returnsErr(RaindexError::OracleFetchError)single.rs): silently continues ← this PR fixes thisFull analysis: #2531
Test plan
Summary by CodeRabbit