enhance error handling #1582
enhance error handling #1582
Conversation
📝 WalkthroughWalkthroughThe TRON client's transaction sending logic was refactored to validate transaction success via the returned Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ 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}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/xchain-tron/src/client.ts (2)
587-593: Apply consistent error handling to theapprove()method.The
approve()method callssendRawTransactionbut doesn't check theresultflag like the updatedtransfer()method does. This inconsistency means approval failures may not be caught or reported properly.Apply this diff:
const signedTx = await this.signTransaction(transaction, walletIndex) -const { txid } = await this.tronWeb.trx.sendRawTransaction(signedTx) - -if (!txid) { - throw new Error('TRC20 Approve Failed') -} - -return txid +const { result, txid, message } = await this.tronWeb.trx.sendRawTransaction(signedTx) +if (!result) { + const errorMessage = message + ? Buffer.from(message, 'hex').toString('utf8') + : 'TRC20 Approve Failed' + throw new Error(errorMessage) +} +return txid
596-599: Apply consistent error handling to thebroadcastTransaction()method.This public method calls
sendRawTransactionwithout checking theresultflag, which is inconsistent with the enhanced error handling added totransfer(). Callers won't receive proper error feedback when broadcasts fail.Apply this diff:
public async broadcastTransaction(signedTx: TronSignedTransaction): Promise<TxHash> { - const { txid } = await this.tronWeb.trx.sendRawTransaction(signedTx) - return txid + const { result, txid, message } = await this.tronWeb.trx.sendRawTransaction(signedTx) + if (!result) { + const errorMessage = message + ? Buffer.from(message, 'hex').toString('utf8') + : 'Transaction broadcast failed' + throw new Error(errorMessage) + } + return txid }
🧹 Nitpick comments (1)
packages/xchain-tron/src/client.ts (1)
483-486: Extract repeated error handling logic into a helper function.The error handling pattern for
sendRawTransactionis duplicated three times. This violates the DRY principle and makes the code harder to maintain.Add a helper method to the class:
private async sendRawTransactionWithErrorHandling(signedTx: TronSignedTransaction): Promise<string> { const { result, txid, message } = await this.tronWeb.trx.sendRawTransaction(signedTx) if (!result) { const errorMessage = message ? Buffer.from(message, 'hex').toString('utf8') : 'Transaction broadcast failed' throw new Error(errorMessage) } return txid }Then replace all three occurrences:
const signedTx = await this.signTransaction(transactionWithMemo, walletIndex) -const { result, txid, message } = await this.tronWeb.trx.sendRawTransaction(signedTx) -if (!result) { - throw new Error(Buffer.from(message, 'hex').toString('utf8')) -} -return txid +return await this.sendRawTransactionWithErrorHandling(signedTx)Also applies to: 491-494, 502-506
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/xchain-tron/src/client.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: mrtommyit
Repo: xchainjs/xchainjs-lib PR: 1536
File: packages/xchain-tron/src/clientKeystore.ts:33-36
Timestamp: 2025-09-16T14:38:22.826Z
Learning: In packages/xchain-tron/src/clientKeystore.ts, the signTransaction method within getSigner() should not have additional error handling wrapped around this.tronWeb.trx.sign() calls, as the method is used internally and the maintainer prefers the current error handling approach.
📚 Learning: 2025-09-16T14:38:22.826Z
Learnt from: mrtommyit
Repo: xchainjs/xchainjs-lib PR: 1536
File: packages/xchain-tron/src/clientKeystore.ts:33-36
Timestamp: 2025-09-16T14:38:22.826Z
Learning: In packages/xchain-tron/src/clientKeystore.ts, the signTransaction method within getSigner() should not have additional error handling wrapped around this.tronWeb.trx.sign() calls, as the method is used internally and the maintainer prefers the current error handling approach.
Applied to files:
packages/xchain-tron/src/client.ts
⏰ 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
| const { result, txid, message } = await this.tronWeb.trx.sendRawTransaction(signedTx) | ||
| if (!result) { | ||
| throw new Error(Buffer.from(message, 'hex').toString('utf8')) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add defensive handling for the error message field.
The code assumes message is always a valid hex-encoded string, but if it's undefined, null, or contains non-hex data, the decoded error message could be empty or produce unexpected output.
Apply this diff to handle edge cases:
const { result, txid, message } = await this.tronWeb.trx.sendRawTransaction(signedTx)
if (!result) {
- throw new Error(Buffer.from(message, 'hex').toString('utf8'))
+ const errorMessage = message
+ ? Buffer.from(message, 'hex').toString('utf8')
+ : 'Transaction broadcast failed'
+ throw new Error(errorMessage)
}🤖 Prompt for AI Agents
In packages/xchain-tron/src/client.ts around lines 483 to 486, the code assumes
message is always a hex string; add defensive handling so that before decoding
you check message is a non-empty string and matches hex characters, decode only
when valid, otherwise fall back to using the raw message or a default like
'Unknown error'; wrap the decode in a try/catch to guard against Buffer.from
errors and construct the thrown Error with the decoded text or the fallback
value so you never attempt to decode undefined/null or non-hex input.
|
looks good, can you run |
2 participants
enhance error handling
Summary by CodeRabbit