fix: improve error handling by replacing silent failures in djed sdk#99
fix: improve error handling by replacing silent failures in djed sdk#99P-ragati wants to merge 2 commits into
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: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe PR standardizes error handling across three SDK utility files by replacing Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
djed-sdk/src/djed/reserveCoin.js (1)
41-42: Keep original exception cause in both reserve-coin price paths.Lines 42 and 71 currently replace underlying failures with generic errors. Attach the original cause so upstream handlers can inspect the real failure.
Proposed patch (apply pattern to both catch blocks)
} catch (error) { console.error("tradeDataPriceBuyRc error:", error); - throw new Error("Failed to calculate reserve coin buy price"); + throw new Error("Failed to calculate reserve coin buy price", { cause: error }); }} catch (error) { console.error("tradeDataPriceSellRc error:", error); - throw new Error("Failed to calculate reserve coin sell price"); + throw new Error("Failed to calculate reserve coin sell price", { cause: error }); }Also applies to: 70-71
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@djed-sdk/src/djed/reserveCoin.js` around lines 41 - 42, The catch blocks in reserveCoin.js currently log the error and throw a new generic Error (messages like "Failed to calculate reserve coin buy price") which loses the original exception; update both catch blocks (the one logging "tradeDataPriceBuyRc error:" and the later one around lines 70-71) to rethrow a new Error that preserves the original error as the cause (e.g., new Error("Failed to calculate reserve coin buy price", { cause: error })) so upstream handlers can inspect the underlying failure, keeping the original console.error logging intact.djed-sdk/src/djed/stableCoin.js (1)
41-42: Retain root-cause data in all stable-coin error rethrows.At Lines 42, 76, and 135, the thrown generic errors hide actionable upstream context. Preserve cause consistently across these three catch blocks.
Proposed patch pattern
-throw new Error("Failed to calculate stable coin buy price"); +throw new Error("Failed to calculate stable coin buy price", { cause: error }); -throw new Error("Failed to calculate stable coin sell price"); +throw new Error("Failed to calculate stable coin sell price", { cause: error }); -throw new Error("Failed to calculate future stable coin price"); +throw new Error("Failed to calculate future stable coin price", { cause: error });Also applies to: 75-76, 134-135
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@djed-sdk/src/djed/stableCoin.js` around lines 41 - 42, In each catch block that currently logs the error and then throws a new generic Error (e.g., the block that logs "tradeDataPriceBuySc error:" and the two other stable-coin catch blocks), preserve the original error by rethrowing with it as the cause—replace throws like throw new Error("Failed to calculate stable coin buy price") with throw new Error("Failed to calculate stable coin buy price", { cause: error }) (and do the analogous change for the other two messages) so upstream code retains the root-cause error; ensure you reference the same human-readable message used in the console.error for each catch block.djed-sdk/src/djed/tradeUtils.js (1)
202-203: Preserve upstream error context when rethrowing.At Line 203, rethrowing a brand-new generic error loses root-cause details for callers. Use
new Error("Failed to fetch fees from contract", { cause: error })to keep the original error attached, maintaining debuggability across the call stack.This syntax is supported in Node.js 16.9.0+. Since all actively supported Node.js versions (20 LTS, 22, 24) include this feature natively, the suggested change is compatible with current runtimes. If your project needs to support Node.js versions older than 16.9.0, use the fallback:
const err = new Error("Failed to fetch fees from contract"); err.cause = error; throw err;Proposed patch
} catch (error) { console.error("getFees error:", error); - throw new Error("Failed to fetch fees from contract"); + throw new Error("Failed to fetch fees from contract", { cause: error }); } };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@djed-sdk/src/djed/tradeUtils.js` around lines 202 - 203, The current catch in getFees logs the error then throws a new generic Error, losing the original cause; modify the rethrow in the getFees function to preserve the upstream error by creating the Error with a cause (e.g., new Error("Failed to fetch fees from contract", { cause: error })) and if you must support older Node versions set err.cause = error before throwing; keep the existing processLogger/console.error line but replace the plain throw with the cause-preserving pattern so callers can inspect error.cause.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@djed-sdk/src/djed/reserveCoin.js`:
- Around line 41-42: The catch blocks in reserveCoin.js currently log the error
and throw a new generic Error (messages like "Failed to calculate reserve coin
buy price") which loses the original exception; update both catch blocks (the
one logging "tradeDataPriceBuyRc error:" and the later one around lines 70-71)
to rethrow a new Error that preserves the original error as the cause (e.g., new
Error("Failed to calculate reserve coin buy price", { cause: error })) so
upstream handlers can inspect the underlying failure, keeping the original
console.error logging intact.
In `@djed-sdk/src/djed/stableCoin.js`:
- Around line 41-42: In each catch block that currently logs the error and then
throws a new generic Error (e.g., the block that logs "tradeDataPriceBuySc
error:" and the two other stable-coin catch blocks), preserve the original error
by rethrowing with it as the cause—replace throws like throw new Error("Failed
to calculate stable coin buy price") with throw new Error("Failed to calculate
stable coin buy price", { cause: error }) (and do the analogous change for the
other two messages) so upstream code retains the root-cause error; ensure you
reference the same human-readable message used in the console.error for each
catch block.
In `@djed-sdk/src/djed/tradeUtils.js`:
- Around line 202-203: The current catch in getFees logs the error then throws a
new generic Error, losing the original cause; modify the rethrow in the getFees
function to preserve the upstream error by creating the Error with a cause
(e.g., new Error("Failed to fetch fees from contract", { cause: error })) and if
you must support older Node versions set err.cause = error before throwing; keep
the existing processLogger/console.error line but replace the plain throw with
the cause-preserving pattern so callers can inspect error.cause.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9a4c913a-b29e-431a-8f62-72eaba629c22
📒 Files selected for processing (3)
djed-sdk/src/djed/reserveCoin.jsdjed-sdk/src/djed/stableCoin.jsdjed-sdk/src/djed/tradeUtils.js
- Attach cause to rethrown errors in reserveCoin, stableCoin, and tradeUtils - Improves debuggability and error traceability
There was a problem hiding this comment.
Pull request overview
This PR aims to improve the djed SDK’s reliability by replacing silent error logging in key pricing/fee helpers with explicit error propagation so callers can handle failures.
Changes:
- Updated
getFeesto log viaconsole.errorand throw a descriptive error instead of silently logging. - Updated stablecoin pricing helpers to throw descriptive errors on failure (buy/sell + future price calculation).
- Updated reservecoin pricing helpers to throw descriptive errors on failure (buy/sell).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| djed-sdk/src/djed/tradeUtils.js | Changes fee-fetching failure handling to throw instead of silently logging. |
| djed-sdk/src/djed/stableCoin.js | Changes stablecoin pricing helpers to throw with clearer error messages. |
| djed-sdk/src/djed/reserveCoin.js | Changes reservecoin pricing helpers to throw with clearer error messages. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } catch (error) { | ||
| console.log("error", error); | ||
| console.error("getFees error:", error); | ||
| throw new Error("Failed to fetch fees from contract", { cause: error }); | ||
| } |
There was a problem hiding this comment.
These changes are only in src/, but djed-sdk/package.json exports dist/umd/index.js + dist/esm/index.js and .npmignore excludes src/. As-is, publishing/consuming the package will not include this improved error handling. Please rebuild and commit the dist/ outputs (or add a build/prepublish step and/or point main/module at the built output generated during publish).
| } catch (error) { | ||
| console.log("calculateFutureScPrice error ", error); | ||
| console.error("calculateFutureScPrice error:", error); | ||
| throw new Error("Failed to calculate future stable coin price", { cause: error }); | ||
| } |
There was a problem hiding this comment.
calculateFutureScPrice calls web3Promise(...), but web3Promise is not imported in this module (only decimalScaling/buildTx are imported from ../helpers). That will cause a ReferenceError at runtime when this function is called (and now gets wrapped by the new throw). Import web3Promise from ../helpers (or reference the correct helper) so the function can actually execute contract calls.
Summary
This PR improves error handling across the djed SDK by replacing silent failures (
console.log) with explicit error propagation.Changes
console.logwithconsole.errorin catch blocksthrow new Error(...)Why this change?
Previously, errors were only logged, which could make debugging difficult and lead to unexpected behavior. This update ensures that errors are properly surfaced to the caller.
Impact
Summary by CodeRabbit
Bug Fixes