Skip to content

Commit

Permalink
fix: use polling on accounts to handle pending connection request (#15)
Browse files Browse the repository at this point in the history
* fix: add pending request handling

* fix: use waitForNextUpdate

* feat: add next distribution channel
  • Loading branch information
VGLoic authored Jan 23, 2022
1 parent e39c698 commit 3d9fee5
Show file tree
Hide file tree
Showing 5 changed files with 90 additions and 20 deletions.
2 changes: 1 addition & 1 deletion .releaserc.yml
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
{
"branches": ["main"]
"branches": ["main", "next"]
}
14 changes: 7 additions & 7 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@

{
"name": "metamask-react",
"version": "0.0.1-semantic-release",
Expand Down Expand Up @@ -57,7 +58,7 @@
"eslint-plugin-react": "^7.22.0",
"eslint-plugin-react-hooks": "^4.2.0",
"eslint-plugin-testing-library": "^3.10.1",
"eth-testing": "^1.0.0-alpha.1",
"eth-testing": "^1.1.0-alpha.4",
"husky": "^4.3.7",
"jest": "^26.6.3",
"lint-staged": "^10.5.3",
Expand Down
34 changes: 34 additions & 0 deletions src/__tests__/index.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,40 @@ describe("MetaMask provider", () => {
expect(result.current.account).toEqual(address);
});

test("calling `connect` method while a pending Metamask request is pending should end in a successful connection", async () => {
const error = {
code: -32002,
};
testingUtils.lowLevel.mockRequest("eth_requestAccounts", error, {
shouldThrow: true,
});

const { result, waitForNextUpdate } = renderHook(useMetaMask, {
wrapper: MetaMaskProvider,
});

expect(result.current.status).toEqual("initializing");

await waitForNextUpdate();

expect(result.current.status).toEqual("notConnected");

act(() => {
result.current.connect();
});

expect(result.current.status).toEqual("connecting");

act(() => {
testingUtils.mockAccounts([address]);
});

await waitForNextUpdate();

expect(result.current.status).toEqual("connected");
expect(result.current.account).toEqual(address);
});

test("calling `connect` method should end in the `notConnected` status if the request fails", async () => {
const error = new Error("Test Error");
testingUtils.lowLevel.mockRequest("eth_requestAccounts", error, {
Expand Down
57 changes: 46 additions & 11 deletions src/metamask-provider.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,13 @@ import {
import { Action, reducer } from "./reducer";
import { useSafeDispatch } from "./utils/useSafeDispatch";

type ErrorWithCode = {
code: number;
[key: string]: any;
};
// MetaMask - RPC Error: Request of type 'wallet_requestPermissions' already pending for origin [origin]. Please wait.
const ERROR_CODE_REQUEST_PENDING = -32002;

type WindowInstanceWithEthereum = Window &
typeof globalThis & { ethereum?: any };

Expand Down Expand Up @@ -63,22 +70,50 @@ function subscribeToChainChanged(dispatch: (action: Action) => void) {
};
}

async function requestAccounts(
function requestAccounts(
dispatch: (action: Action) => void
): Promise<string[]> {
const ethereum = (window as WindowInstanceWithEthereum).ethereum;

dispatch({ type: "metaMaskConnecting" });
try {
const accounts: string[] = await ethereum.request({
method: "eth_requestAccounts",
});
dispatch({ type: "metaMaskConnected", payload: { accounts } });
return accounts;
} catch (err) {
dispatch({ type: "metaMaskPermissionRejected" });
throw err;
}

/**
* Note about the pattern
* Instead of only relying on the RPC Request response, the resolve of the promise may happen based from a polling
* using the eth_accounts rpc endpoint.
* The reason for this change is in order to handle pending connection request on MetaMask side.
* See https://github.com/VGLoic/metamask-react/issues/13 for the full discussion.
* Any improvements on MetaMask side on this behaviour that could allow to go back to the previous, simple and safer, pattern
* should trigger the update of this code.
*/

return new Promise((resolve, reject) => {
const intervalId = setInterval(async () => {
const accounts = await ethereum.request({
method: "eth_accounts",
});
if (accounts.length > 0) {
clearInterval(intervalId);
dispatch({ type: "metaMaskConnected", payload: { accounts } });
resolve(accounts);
}
}, 500);
ethereum
.request({
method: "eth_requestAccounts",
})
.then((accounts: string[]) => {
clearInterval(intervalId);
dispatch({ type: "metaMaskConnected", payload: { accounts } });
resolve(accounts);
})
.catch((err: unknown) => {
if ((err as ErrorWithCode)?.code === ERROR_CODE_REQUEST_PENDING) return;
dispatch({ type: "metaMaskPermissionRejected" });
clearInterval(intervalId);
reject(err);
});
});
}

const initialState: MetaMaskState = {
Expand Down

0 comments on commit 3d9fee5

Please sign in to comment.