Fix: Initialize Web3 with injected provider when MetaMask is available#89
Fix: Initialize Web3 with injected provider when MetaMask is available#89Rav1Chauhan wants to merge 1 commit intoDjedAlliance:mainfrom
Conversation
📝 WalkthroughWalkthroughThe pull request refactors the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 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.
Actionable comments posted: 3
🧹 Nitpick comments (1)
djed-sdk/src/web3.js (1)
6-9: Consider requesting accounts before constructing the Web3 instance.The current order creates a
Web3instance on line 6 and then requests account access on line 9. If the user denies, the Web3 instance is silently garbage-collected — functionally harmless, but inverted from the natural read order (access → instance). Flipping the two ensuresWeb3is only constructed when a grant is confirmed.♻️ Suggested reorder
- const web3 = new Web3(window.ethereum); - - // Request account access (important) - await window.ethereum.request({ method: "eth_requestAccounts" }); - - return web3; + // Request account access before constructing the instance + await window.ethereum.request({ method: "eth_requestAccounts" }); + + return new Web3(window.ethereum);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@djed-sdk/src/web3.js` around lines 6 - 9, Request accounts with window.ethereum.request({ method: "eth_requestAccounts" }) before constructing the Web3 instance so Web3 is only created after permission is granted; move the await window.ethereum.request call to before const web3 = new Web3(window.ethereum), and ensure any rejection from window.ethereum.request is handled (e.g., try/catch or .catch) so you only call new Web3(window.ethereum) on success.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@djed-sdk/src/web3.js`:
- Around line 12-14: The catch block currently logs a hardcoded "User denied
account access" for all errors; update the catch to inspect the error (e.g.,
check error.code === 4001) and only log the user-rejection message when that
code is present, otherwise log a generic or the actual error (include
error.message or error) to avoid misleading output; locate the catch around the
window.ethereum.request (or provider request) call and replace the unconditional
console.error("User denied account access", error) with a conditional log based
on error.code.
- Line 19: The getWeb3 function currently constructs new
Web3.providers.HttpProvider(URI) without validating URI, which can pass
undefined and produce opaque provider errors; update getWeb3 to validate the URI
parameter (ensure it's a non-empty string and a valid URL scheme like
http/https) before creating a new HttpProvider, and if invalid or missing (and
window.ethereum is absent) throw or return a clear error (e.g., throw new
Error('Missing or invalid RPC URI')) so callers get an immediate, descriptive
failure instead of opaque provider-level errors; locate the check around
getWeb3, the URI variable, and the Web3.providers.HttpProvider instantiation to
implement this guard.
- Around line 3-19: The published bundle still contains the old getWeb3
implementation, so rebuild and overwrite the dist artifacts with the fixed
source: run the project build step (so dist/esm/index.js and dist/umd/index.js
are regenerated) ensuring the exported getWeb3 function in the dist matches the
source that uses window.ethereum and eth_requestAccounts; verify package.json's
"main" and "module" still point to dist/umd/index.js and dist/esm/index.js and
then commit the regenerated files (or update the build/publish pipeline to run
the build before publishing) so consumers receive the corrected implementation.
---
Nitpick comments:
In `@djed-sdk/src/web3.js`:
- Around line 6-9: Request accounts with window.ethereum.request({ method:
"eth_requestAccounts" }) before constructing the Web3 instance so Web3 is only
created after permission is granted; move the await window.ethereum.request call
to before const web3 = new Web3(window.ethereum), and ensure any rejection from
window.ethereum.request is handled (e.g., try/catch or .catch) so you only call
new Web3(window.ethereum) on success.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
stablepay-sdk/dist/umd/index.js.mapis excluded by!**/dist/**,!**/*.mapstablepay-sdk/dist/umd/test.htmlis excluded by!**/dist/**stablepay-sdk/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (2)
djed-sdk/src/web3.jsstablepay-sdk/.gitignore
| export const getWeb3 = async (URI) => { | ||
| if (typeof window !== "undefined" && window.ethereum) { | ||
| try { | ||
| const web3 = new Web3(window.ethereum); | ||
|
|
||
| // Request account access (important) | ||
| await window.ethereum.request({ method: "eth_requestAccounts" }); | ||
|
|
||
| return web3; | ||
| } catch (error) { | ||
| console.error("User denied account access", error); | ||
| throw error; | ||
| } | ||
| }); | ||
| } | ||
|
|
||
| // Fallback to RPC | ||
| return new Web3(new Web3.providers.HttpProvider(URI)); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 1. Check if djed-sdk/dist files are tracked by git
echo "=== Checking if djed-sdk/dist files are tracked by git ==="
git -C djed-sdk ls-files dist/ 2>/dev/null | head -20
# 2. Confirm what entrypoints package.json exposes
echo ""
echo "=== Checking djed-sdk/package.json entrypoints ==="
fd "package.json" djed-sdk --max-depth 2 --exec cat {}
# 3. Show the stale provider logic still in dist
echo ""
echo "=== Searching for BLOCKCHAIN_URI or old Web3 logic in dist ==="
rg -n "BLOCKCHAIN_URI\|new Web3\(BLOCKCHAIN_URI" djed-sdk/dist/ 2>/dev/null || echo "No matches found"Repository: DjedAlliance/StablePay
Length of output: 836
🏁 Script executed:
#!/bin/bash
# Check the actual content of dist files to see if they're truly stale
echo "=== Content of dist/esm/index.js ==="
cat djed-sdk/dist/esm/index.js
echo ""
echo "=== Content of dist/umd/index.js ==="
cat djed-sdk/dist/umd/index.js
# Also check git history to see when dist was last updated
echo ""
echo "=== Recent git commits touching dist files ==="
git log --oneline -n 10 -- djed-sdk/dist/Repository: DjedAlliance/StablePay
Length of output: 50378
Dist bundles in djed-sdk/dist/ are stale and ship the old broken implementation.
The dist files (djed-sdk/dist/esm/index.js and djed-sdk/dist/umd/index.js) still contain the original broken getWeb3 implementation that directly passes BLOCKCHAIN_URI to Web3(BLOCKCHAIN_URI), whereas the fixed source code properly requests account access and uses window.ethereum. Since package.json points to these dist files as the main entrypoints ("main": "dist/umd/index.js", "module": "dist/esm/index.js"), consumers will receive the broken code from the published package regardless of this source fix.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@djed-sdk/src/web3.js` around lines 3 - 19, The published bundle still
contains the old getWeb3 implementation, so rebuild and overwrite the dist
artifacts with the fixed source: run the project build step (so
dist/esm/index.js and dist/umd/index.js are regenerated) ensuring the exported
getWeb3 function in the dist matches the source that uses window.ethereum and
eth_requestAccounts; verify package.json's "main" and "module" still point to
dist/umd/index.js and dist/esm/index.js and then commit the regenerated files
(or update the build/publish pipeline to run the build before publishing) so
consumers receive the corrected implementation.
| } catch (error) { | ||
| console.error("User denied account access", error); | ||
| throw error; |
There was a problem hiding this comment.
Catch-all error message misleadingly implies the error is always a user denial.
"User denied account access" is only accurate for the EIP-1193 user-rejection code (4001). The same catch block fires for any runtime error (e.g., window.ethereum.request missing, unexpected provider failures). Either narrow the message or inspect the error code before logging.
🛡️ Suggested fix
- console.error("User denied account access", error);
+ console.error("Failed to request account access", error);Or, if you want to preserve the specific message for user rejections:
} catch (error) {
+ if (error.code === 4001) {
+ console.error("User denied account access", error);
+ } else {
console.error("User denied account access", error);
+ }
throw error;
}Simplified:
- console.error("User denied account access", error);
+ const msg = error?.code === 4001
+ ? "User denied account access"
+ : "Failed to initialize wallet provider";
+ console.error(msg, error);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| } catch (error) { | |
| console.error("User denied account access", error); | |
| throw error; | |
| } catch (error) { | |
| const msg = error?.code === 4001 | |
| ? "User denied account access" | |
| : "Failed to initialize wallet provider"; | |
| console.error(msg, error); | |
| throw error; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@djed-sdk/src/web3.js` around lines 12 - 14, The catch block currently logs a
hardcoded "User denied account access" for all errors; update the catch to
inspect the error (e.g., check error.code === 4001) and only log the
user-rejection message when that code is present, otherwise log a generic or the
actual error (include error.message or error) to avoid misleading output; locate
the catch around the window.ethereum.request (or provider request) call and
replace the unconditional console.error("User denied account access", error)
with a conditional log based on error.code.
| } | ||
|
|
||
| // Fallback to RPC | ||
| return new Web3(new Web3.providers.HttpProvider(URI)); |
There was a problem hiding this comment.
URI is not validated before being passed to HttpProvider.
If a caller invokes getWeb3() without a URI argument and window.ethereum is absent, new Web3.providers.HttpProvider(undefined) is constructed silently. All subsequent RPC calls will fail with opaque provider-level errors rather than a clear early-rejection.
🛡️ Suggested guard
+ if (!URI) {
+ throw new Error("No injected provider found and no fallback URI supplied.");
+ }
// Fallback to RPC
return new Web3(new Web3.providers.HttpProvider(URI));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| return new Web3(new Web3.providers.HttpProvider(URI)); | |
| if (!URI) { | |
| throw new Error("No injected provider found and no fallback URI supplied."); | |
| } | |
| // Fallback to RPC | |
| return new Web3(new Web3.providers.HttpProvider(URI)); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@djed-sdk/src/web3.js` at line 19, The getWeb3 function currently constructs
new Web3.providers.HttpProvider(URI) without validating URI, which can pass
undefined and produce opaque provider errors; update getWeb3 to validate the URI
parameter (ensure it's a non-empty string and a valid URL scheme like
http/https) before creating a new HttpProvider, and if invalid or missing (and
window.ethereum is absent) throw or return a clear error (e.g., throw new
Error('Missing or invalid RPC URI')) so callers get an immediate, descriptive
failure instead of opaque provider-level errors; locate the check around
getWeb3, the URI variable, and the Web3.providers.HttpProvider instantiation to
implement this guard.
1 participant
Addressed Issues:
Fixes #63
This PR fixes the Web3 initialization logic inside the SDK.
Previously, the SDK detected window.ethereum (MetaMask) but still instantiated Web3 using a static HttpProvider(URI). As a result, state-changing transactions did not trigger the MetaMask popup because the injected provider was never used.
Changes Made
Updated getWeb3() to initialize Web3 using:
new Web3(window.ethereum)
when window.ethereum is available.
Added eth_requestAccounts to properly request wallet access.
Preserved fallback behavior to HttpProvider(URI) when no injected provider exists.
Result
MetaMask popup now appears for state-changing transactions.
Transactions are correctly signed using the injected provider.
No breaking changes introduced.
Fallback behavior remains intact.
Additional Notes:
Checklist
We encourage contributors to use AI tools responsibly when creating Pull Requests. While AI can be a valuable aid, it is essential to ensure that your contributions meet the task requirements, build successfully, include relevant tests, and pass all linters. Submissions that do not meet these standards may be closed without warning to maintain the quality and integrity of the project. Please take the time to understand the changes you are proposing and their impact.
Summary by CodeRabbit
Refactor
Chores