fix: improve return data formatting in readContract #909
Conversation
… stringify and JSON.parse
WalkthroughThe change updates the response handling in the Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Note 🔌 MCP (Model Context Protocol) integration is now available in Early Access!Pro users can now connect to remote MCP servers under the Integrations page to get reviews and chat conversations that understand additional development context. ✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/server/routes/contract/read/read.ts(2 hunks)
⏰ 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). (2)
- GitHub Check: build
- GitHub Check: lint
🔇 Additional comments (1)
src/server/routes/contract/read/read.ts (1)
6-6: LGTM!The import addition is necessary for the stringify utility used later in the code.
| reply.status(StatusCodes.OK).send({ | ||
| // biome-ignore lint/suspicious/noExplicitAny: data from chain | ||
| result: returnData as any, | ||
| result: JSON.parse(stringify(returnData)) as any, |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify the necessity of the serialize-deserialize pattern.
The code now performs JSON.parse(stringify(returnData)) which seems redundant since the data is immediately stringified and then parsed back. This pattern, combined with the existing bigNumberReplacer on line 99, raises several concerns:
- Double processing: Both
bigNumberReplacerandstringify/parsemay be handling similar data transformations - Performance overhead: The serialize-deserialize cycle adds unnecessary computational cost
- Potential conflicts: The two transformations might interfere with each other
Consider if this pattern is truly necessary or if one of these approaches would be better:
- Use only
bigNumberReplacerif it handles all required transformations - Use only
stringifyif it's more comprehensive thanbigNumberReplacer - Apply transformations in a single pass
Let me verify what the stringify function does and if this pattern is commonly used:
🏁 Script executed:
#!/bin/bash
# Search for similar stringify/parse patterns in the codebase
rg -A 2 -B 2 "JSON\.parse.*stringify"
# Search for other usages of the stringify utility
rg -A 5 -B 2 "stringify.*from.*thirdweb"
# Search for bigNumberReplacer usage patterns
rg -A 3 -B 3 "bigNumberReplacer"Length of output: 9739
Review JSON serialization in read.ts
It looks like we first run returnData = bigNumberReplacer(returnData) and then immediately do
result: JSON.parse(stringify(returnData)) as any– a pattern also used in get-receipt.ts and admin/transaction.ts to deep-clone and sanitize outputs. Since stringify from thirdweb/utils already applies its own BigNumber/BigInt replacer, this double handling may be redundant and adds extra CPU cost.
Please consider one of the following:
- Drop the explicit
bigNumberReplacercall and rely solely onstringify’s built-in replacer. - Or skip the
JSON.parse(stringify(...))step and sendreturnDatadirectly after runningbigNumberReplacer. - If both steps are required for edge cases, add a comment explaining why.
Location to update:
• src/server/routes/contract/read/read.ts, line 103 (result: JSON.parse(stringify(returnData)) as any)
🤖 Prompt for AI Agents
In src/server/routes/contract/read/read.ts at line 103, the code redundantly
applies bigNumberReplacer to returnData before using stringify from
thirdweb/utils, which already handles BigNumber/BigInt serialization. To fix
this, remove the explicit bigNumberReplacer call and rely solely on stringify's
built-in replacer, then keep the JSON.parse(stringify(returnData)) step for deep
cloning and sanitization. Alternatively, if you prefer to keep both steps, add a
clear comment explaining why both are necessary to avoid confusion.
PR-Codex overview
This PR updates the
readContractfunction in thesrc/server/routes/contract/read/read.tsfile to enhance the handling ofreturnDataby converting it to a JSON string and then parsing it back, improving data consistency.Detailed summary
stringifyimport fromthirdweb/utils.resultassignment in the response to useJSON.parse(stringify(returnData))instead ofreturnData as any.Summary by CodeRabbit