Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: multipl session disconnects throwing #2655

Merged
merged 1 commit into from
Oct 19, 2023
Merged

Conversation

iamacook
Copy link
Member

What it solves

Resolves multiple session disconnects throwing

How this PR fixes it

Session updated are now handled one after each other.

How to test it

Connect two DApps that support chain A but not chain B. Switch from chain A to chain B and observe no WC-related error.

Checklist

  • I've tested the branch on mobile 📱
  • I've documented how it affects the analytics (if at all) 📊
  • I've written a unit/e2e test for it (if applicable) 🧑‍💻

@iamacook iamacook self-assigned this Oct 18, 2023
@github-actions
Copy link

github-actions bot commented Oct 18, 2023

Branch preview

✅ Deploy successful!

https://fix_multiple_disconnection--walletweb.review-wallet-web.5afe.dev

Comment on lines +144 to +153
const isUnsupportedChain = !currentEip155ChainIds.includes(newEip155ChainId)
const isNewSessionSafe = !currentEip155Accounts.includes(newEip155Account)

// Switching to unsupported chain
if (hasNewChainId) {
if (isUnsupportedChain) {
return this.disconnectSession(session)
}

// Add new account to the session namespace
if (hasNewAccount) {
// Add new Safe to the session namespace
if (isNewSessionSafe) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These renames somehow didn't merge correctly before.

@github-actions
Copy link

ESLint Summary View Full Report

Annotations are provided inline on the Files Changed tab. You can also see all annotations that were generated on the annotations page.

Type Occurrences Fixable
Errors 0 0
Warnings 0 0
Ignored 0 N/A
  • Result: ✅ success
  • Annotations: 0 total

Report generated by eslint-plus-action

@github-actions
Copy link

Coverage report

❌ An unexpected error occurred. For more details, check console

Error: The process '/usr/local/bin/yarn' failed with exit code 1
St.
Category Percentage Covered / Total
🟡 Statements 75% 9183/12244
🔴 Branches 49.21% 1877/3814
🔴 Functions 57.17% 1356/2372
🟡 Lines 76.62% 8306/10840

Test suite run failed

Failed tests: 2/1010. Failed suites: 4/141.
  ● Test suite failed to run

    Jest encountered an unexpected token

    Jest failed to parse a file. This happens e.g. when your code or its dependencies use non-standard JavaScript syntax, or when Jest is not configured to support such syntax.

    Out of the box Jest supports Babel, which will be used to transform your files into valid JS based on your Babel configuration.

    By default "node_modules" folder is ignored by transformers.

    Here's what you can do:
     • If you are trying to use ECMAScript Modules, see https://jestjs.io/docs/ecmascript-modules for how to enable it.
     • If you are trying to use TypeScript, see https://jestjs.io/docs/getting-started#using-typescript
     • To have some of your "node_modules" files transformed, you can specify a custom "transformIgnorePatterns" in your config.
     • If you need a custom transformation specify a "transform" option in your config.
     • If you simply want to mock your non-JS modules (e.g. binary assets) you can stub them out with the "moduleNameMapper" config option.

    You'll find more details and examples of these config options in the docs:
    https://jestjs.io/docs/configuration
    For information about custom transformations, see:
    https://jestjs.io/docs/code-transformation

    Details:

    /home/runner/work/safe-wallet-web/safe-wallet-web/node_modules/multiformats/esm/src/basics.js:1
    ({"Object.<anonymous>":function(module,exports,require,__dirname,__filename,jest){import * as identityBase from './bases/identity.js';
                                                                                      ^^^^^^

    SyntaxError: Cannot use import statement outside a module



      at Runtime.createScriptFromCode (node_modules/jest-runtime/build/index.js:1505:14)
      at Object.<anonymous> (node_modules/uint8arrays/esm/src/util/bases.js:11:17)
      at Object.<anonymous> (node_modules/uint8arrays/esm/src/from-string.js:11:55)
      at Object.<anonymous> (node_modules/uint8arrays/esm/src/index.js:34:21)
      at Object.<anonymous> (node_modules/@walletconnect/utils/dist/index.cjs.js:1:344)
      at Object.<anonymous> (src/services/walletconnect/WalletConnectContext.tsx:1203:27)
      at Object.<anonymous> (src/services/walletconnect/__tests__/WalletConnectContext.test.tsx:11:31)


  ● Test suite failed to run

    Jest encountered an unexpected token

    Jest failed to parse a file. This happens e.g. when your code or its dependencies use non-standard JavaScript syntax, or when Jest is not configured to support such syntax.

    Out of the box Jest supports Babel, which will be used to transform your files into valid JS based on your Babel configuration.

    By default "node_modules" folder is ignored by transformers.

    Here's what you can do:
     • If you are trying to use ECMAScript Modules, see https://jestjs.io/docs/ecmascript-modules for how to enable it.
     • If you are trying to use TypeScript, see https://jestjs.io/docs/getting-started#using-typescript
     • To have some of your "node_modules" files transformed, you can specify a custom "transformIgnorePatterns" in your config.
     • If you need a custom transformation specify a "transform" option in your config.
     • If you simply want to mock your non-JS modules (e.g. binary assets) you can stub them out with the "moduleNameMapper" config option.

    You'll find more details and examples of these config options in the docs:
    https://jestjs.io/docs/configuration
    For information about custom transformations, see:
    https://jestjs.io/docs/code-transformation

    Details:

    /home/runner/work/safe-wallet-web/safe-wallet-web/node_modules/multiformats/esm/src/basics.js:1
    ({"Object.<anonymous>":function(module,exports,require,__dirname,__filename,jest){import * as identityBase from './bases/identity.js';
                                                                                      ^^^^^^

    SyntaxError: Cannot use import statement outside a module



      at Runtime.createScriptFromCode (node_modules/jest-runtime/build/index.js:1505:14)
      at Object.<anonymous> (node_modules/uint8arrays/esm/src/util/bases.js:11:17)
      at Object.<anonymous> (node_modules/uint8arrays/esm/src/from-string.js:11:55)
      at Object.<anonymous> (node_modules/uint8arrays/esm/src/index.js:34:21)
      at Object.<anonymous> (node_modules/@walletconnect/utils/dist/index.cjs.js:1:344)
      at Object.<anonymous> (src/services/walletconnect/WalletConnectWallet.ts:2066:26)
      at Object.<anonymous> (src/services/walletconnect/__tests__/WalletConnectWallet.test.ts:38:69)


  ● useCompatibilityWarning › should return an error for a dangerous bridge

    TypeError: Cannot destructure property 'message' of '((cov_2igsvcx892(...).s[17]++) , (intermediate value)(intermediate value)(intermediate value))' as it is undefined.

      40 |     const { proposer } = proposal.params
      41 |
    > 42 |     let { message, severity } = isStrictAddressBridge(origin)
         |           ^
      43 |       ? Warnings.DANGEROUS_BRIDGE
      44 |       : isDefaultAddressBridge(origin)
      45 |       ? Warnings.RISKY_BRIDGE

      at message (src/components/walletconnect/ProposalForm/useCompatibilityWarning.ts:42:11)
      at mountMemo (node_modules/react-dom/cjs/react-dom.development.js:17225:19)
      at Object.useMemo (node_modules/react-dom/cjs/react-dom.development.js:17670:16)
      at useMemo (node_modules/react/cjs/react.development.js:1650:21)
      at useCompatibilityWarning (src/components/walletconnect/ProposalForm/useCompatibilityWarning.ts:38:17)
      at src/components/walletconnect/ProposalForm/__tests__/useCompatibilityWarning.test.ts:17:64
      at TestComponent (node_modules/@testing-library/react/dist/pure.js:283:27)
      at renderWithHooks (node_modules/react-dom/cjs/react-dom.development.js:16305:18)
      at mountIndeterminateComponent (node_modules/react-dom/cjs/react-dom.development.js:20074:13)
      at beginWork (node_modules/react-dom/cjs/react-dom.development.js:21587:16)
      at beginWork$1 (node_modules/react-dom/cjs/react-dom.development.js:27426:14)
      at performUnitOfWork (node_modules/react-dom/cjs/react-dom.development.js:26560:12)
      at workLoopSync (node_modules/react-dom/cjs/react-dom.development.js:26466:5)
      at renderRootSync (node_modules/react-dom/cjs/react-dom.development.js:26434:7)
      at recoverFromConcurrentError (node_modules/react-dom/cjs/react-dom.development.js:25850:20)
      at performConcurrentWorkOnRoot (node_modules/react-dom/cjs/react-dom.development.js:25750:22)
      at flushActQueue (node_modules/react/cjs/react.development.js:2667:24)
      at act (node_modules/react/cjs/react.development.js:2582:11)
      at node_modules/@testing-library/react/dist/act-compat.js:63:25
      at renderRoot (node_modules/@testing-library/react/dist/pure.js:159:26)
      at render (node_modules/@testing-library/react/dist/pure.js:246:10)
      at renderHook (node_modules/@testing-library/react/dist/pure.js:293:7)
      at customRenderHook (src/tests/test-utils.tsx:81:20)
      at Object.<anonymous> (src/components/walletconnect/ProposalForm/__tests__/useCompatibilityWarning.test.ts:17:34)

  ● useCompatibilityWarning › should return a warning for a risky bridge

    TypeError: Cannot destructure property 'message' of '((cov_2igsvcx892(...).s[17]++) , (intermediate value)(intermediate value)(intermediate value))' as it is undefined.

      40 |     const { proposer } = proposal.params
      41 |
    > 42 |     let { message, severity } = isStrictAddressBridge(origin)
         |           ^
      43 |       ? Warnings.DANGEROUS_BRIDGE
      44 |       : isDefaultAddressBridge(origin)
      45 |       ? Warnings.RISKY_BRIDGE

      at message (src/components/walletconnect/ProposalForm/useCompatibilityWarning.ts:42:11)
      at mountMemo (node_modules/react-dom/cjs/react-dom.development.js:17225:19)
      at Object.useMemo (node_modules/react-dom/cjs/react-dom.development.js:17670:16)
      at useMemo (node_modules/react/cjs/react.development.js:1650:21)
      at useCompatibilityWarning (src/components/walletconnect/ProposalForm/useCompatibilityWarning.ts:38:17)
      at src/components/walletconnect/ProposalForm/__tests__/useCompatibilityWarning.test.ts:35:64
      at TestComponent (node_modules/@testing-library/react/dist/pure.js:283:27)
      at renderWithHooks (node_modules/react-dom/cjs/react-dom.development.js:16305:18)
      at mountIndeterminateComponent (node_modules/react-dom/cjs/react-dom.development.js:20074:13)
      at beginWork (node_modules/react-dom/cjs/react-dom.development.js:21587:16)
      at beginWork$1 (node_modules/react-dom/cjs/react-dom.development.js:27426:14)
      at performUnitOfWork (node_modules/react-dom/cjs/react-dom.development.js:26560:12)
      at workLoopSync (node_modules/react-dom/cjs/react-dom.development.js:26466:5)
      at renderRootSync (node_modules/react-dom/cjs/react-dom.development.js:26434:7)
      at recoverFromConcurrentError (node_modules/react-dom/cjs/react-dom.development.js:25850:20)
      at performConcurrentWorkOnRoot (node_modules/react-dom/cjs/react-dom.development.js:25750:22)
      at flushActQueue (node_modules/react/cjs/react.development.js:2667:24)
      at act (node_modules/react/cjs/react.development.js:2582:11)
      at node_modules/@testing-library/react/dist/act-compat.js:63:25
      at renderRoot (node_modules/@testing-library/react/dist/pure.js:159:26)
      at render (node_modules/@testing-library/react/dist/pure.js:246:10)
      at renderHook (node_modules/@testing-library/react/dist/pure.js:293:7)
      at customRenderHook (src/tests/test-utils.tsx:81:20)
      at Object.<anonymous> (src/components/walletconnect/ProposalForm/__tests__/useCompatibilityWarning.test.ts:35:34)


  ● Test suite failed to run

    Jest encountered an unexpected token

    Jest failed to parse a file. This happens e.g. when your code or its dependencies use non-standard JavaScript syntax, or when Jest is not configured to support such syntax.

    Out of the box Jest supports Babel, which will be used to transform your files into valid JS based on your Babel configuration.

    By default "node_modules" folder is ignored by transformers.

    Here's what you can do:
     • If you are trying to use ECMAScript Modules, see https://jestjs.io/docs/ecmascript-modules for how to enable it.
     • If you are trying to use TypeScript, see https://jestjs.io/docs/getting-started#using-typescript
     • To have some of your "node_modules" files transformed, you can specify a custom "transformIgnorePatterns" in your config.
     • If you need a custom transformation specify a "transform" option in your config.
     • If you simply want to mock your non-JS modules (e.g. binary assets) you can stub them out with the "moduleNameMapper" config option.

    You'll find more details and examples of these config options in the docs:
    https://jestjs.io/docs/configuration
    For information about custom transformations, see:
    https://jestjs.io/docs/code-transformation

    Details:

    /home/runner/work/safe-wallet-web/safe-wallet-web/node_modules/multiformats/esm/src/basics.js:1
    ({"Object.<anonymous>":function(module,exports,require,__dirname,__filename,jest){import * as identityBase from './bases/identity.js';
                                                                                      ^^^^^^

    SyntaxError: Cannot use import statement outside a module



      at Runtime.createScriptFromCode (node_modules/jest-runtime/build/index.js:1505:14)
      at Object.<anonymous> (node_modules/uint8arrays/esm/src/util/bases.js:11:17)
      at Object.<anonymous> (node_modules/uint8arrays/esm/src/from-string.js:11:55)
      at Object.<anonymous> (node_modules/uint8arrays/esm/src/index.js:34:21)
      at Object.<anonymous> (node_modules/@walletconnect/utils/dist/index.cjs.js:1:344)
      at Object.<anonymous> (src/services/walletconnect/WalletConnectContext.tsx:1203:27)
      at Object.<anonymous> (src/services/walletconnect/__tests__/useWalletConnectSessions.test.tsx:7:31)

Report generated by 🧪jest coverage report action from ab82d11

@iamacook iamacook merged commit ec255f4 into nect Oct 19, 2023
7 of 10 checks passed
@iamacook iamacook deleted the fix-multiple-disconnection branch October 19, 2023 06:51
@github-actions github-actions bot locked and limited conversation to collaborators Oct 19, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants