fix: missing prices #1079
fix: missing prices #1079
Conversation
|
Warning Rate limit exceeded@chedieck has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 14 minutes and 28 seconds before requesting another review. ⌛ 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. 📒 Files selected for processing (1)
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. WalkthroughAdded price sync call to the blockchain worker's initialization phase, enhanced logging in the price service with human-readable date formatting, and updated Docker Compose environment variables from MySQL to MariaDB naming conventions. Changes
Sequence DiagramsequenceDiagram
participant Worker as syncBlockchainAndPricesWorker
participant PriceService as priceService
participant Logger as Logger
Worker->>PriceService: syncPastDaysNewerPrices() [NEW]
PriceService->>Logger: Log last price date (human-readable)
PriceService->>Logger: Log count of prices to retrieve
PriceService-->>Worker: Return
Worker->>Worker: syncMissedTransactions()
Worker->>Worker: connectPrices()
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 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 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
services/priceService.ts
Outdated
| const date = moment().startOf('day') | ||
| const daysToRetrieve: string[] = [] | ||
|
|
||
| console.log(`[PRICES] Last price found is for ${date.format(HUMAN_READABLE_DATE_FORMAT)}.`) |
There was a problem hiding this comment.
logic: Logging wrong variable - should log lastDateInDB not date
| console.log(`[PRICES] Last price found is for ${date.format(HUMAN_READABLE_DATE_FORMAT)}.`) | |
| console.log(`[PRICES] Last price found is for ${lastDateInDB.format(HUMAN_READABLE_DATE_FORMAT)}.`) |
Prompt To Fix With AI
This is a comment left during a code review.
Path: services/priceService.ts
Line: 168:168
Comment:
**logic:** Logging wrong variable - should log `lastDateInDB` not `date`
```suggestion
console.log(`[PRICES] Last price found is for ${lastDateInDB.format(HUMAN_READABLE_DATE_FORMAT)}.`)
```
How can I resolve this? If you propose a fix, please make it concise.![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
🧹 Nitpick comments (1)
jobs/workers.ts (1)
38-41: Update the log message to reflect all operations.The log message on line 38 says "syncing missed transactions and connecting prices..." but now the worker also syncs past prices at the start. Consider updating it to accurately describe all three operations.
Apply this diff to update the log message:
- console.log(`job ${job.id as string}: syncing missed transactions and connecting prices...`) + console.log(`job ${job.id as string}: syncing past prices, missed transactions, and connecting prices...`) await priceService.syncPastDaysNewerPrices()Additionally, the new execution order (sync prices → sync transactions → connect prices) makes sense because it ensures historical prices are up-to-date before processing transactions. However, note that if
syncPastDaysNewerPrices()fails, the entire job will fail and subsequent operations won't execute. This appears to be the intended behavior given the existing error handling.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
jobs/workers.ts(1 hunks)services/priceService.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
services/priceService.ts (1)
constants/index.ts (2)
HUMAN_READABLE_DATE_FORMAT(165-165)PRICE_API_DATE_FORMAT(167-167)
⏰ 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: Run Tests
🔇 Additional comments (2)
services/priceService.ts (2)
5-5: LGTM!The import of
HUMAN_READABLE_DATE_FORMATis correctly added and used in the logging statement below.
173-173: LGTM!The log correctly reports the number of prices that will be retrieved, improving observability of the sync operation.
chedieck
force-pushed
the
fix/missing-prices2
branch
from
f11bc39 to
acb1744
Compare
Summary by CodeRabbit
Bug Fixes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.
Greptile Overview
Greptile Summary
Fixed missing prices by ensuring
syncPastDaysNewerPrices()runs before connecting transactions to prices in the blockchain sync worker. This prevents errors whenconnectAllTransactionsToPrices()attempts to link transactions to price data that doesn't exist yet.syncPastDaysNewerPrices()call at the start ofsyncBlockchainAndPricesWorkerjobHUMAN_READABLE_DATE_FORMATconstant for improved log formattingIssue found: Line 168 in
priceService.tslogs the wrong variable - it outputs today's date instead of the actual last price date from the database.Confidence Score: 3/5
Important Files Changed
File Analysis
syncPastDaysNewerPrices()call before syncing transactions to ensure required price data existsHUMAN_READABLE_DATE_FORMATand logging statements. Critical bug: line 168 logs wrong variableSequence Diagram
sequenceDiagram participant Worker as syncBlockchainAndPricesWorker participant PriceService as priceService participant BlockchainClient as multiBlockchainClient participant TxService as transactionService participant DB as Database Worker->>PriceService: syncPastDaysNewerPrices() PriceService->>DB: Query last price timestamp DB-->>PriceService: lastPrice.timestamp PriceService->>PriceService: Calculate missing days PriceService->>PriceService: Log: Last price date (bug: logs today instead) PriceService->>PriceService: getAllPricesByNetworkTicker(XEC) PriceService->>PriceService: getAllPricesByNetworkTicker(BCH) PriceService->>DB: Upsert missing prices DB-->>PriceService: Success PriceService-->>Worker: Prices synced Worker->>BlockchainClient: syncMissedTransactions() BlockchainClient->>DB: Insert/update transactions DB-->>BlockchainClient: Success BlockchainClient-->>Worker: Transactions synced Worker->>TxService: connectAllTransactionsToPrices() TxService->>DB: Fetch transactions with no/irregular prices DB-->>TxService: Transaction list TxService->>DB: Bulk fetch prices for (networkId, timestamp) pairs DB-->>TxService: Price data alt Prices exist TxService->>DB: Create price-transaction links DB-->>TxService: Success else Prices missing TxService-->>Worker: Error thrown end TxService-->>Worker: Prices connected