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

feat: active session management for stale / removed sessions #356

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

kantorcodes
Copy link
Contributor

@kantorcodes kantorcodes commented Nov 25, 2024

Description:

There have been reports that sessions no longer exist despite a Signer still being available. This PR introduces a more active role in the library to automatically detect this kind of a mismatch.

  • It will remove a Signer when its session is no longer valid. A session might become invalid either by expiring, by a user disconnecting from the wallet, or some sort of a desync from IndexedDB.
  • Adds better typing for Log classes, and disables logs during test:watch to make it easier to debug tests.

Related issue(s):

Fixes #

Notes for reviewer:

Checklist

  • Documented (Code comments, README, etc.)
  • Tested (unit, integration, etc.)

Copy link

github-actions bot commented Nov 25, 2024

Coverage report

St.
Category Percentage Covered / Total
🟡 Statements
78.64% (+1.19% 🔼)
519/660
🟡 Branches
64.54% (+2.04% 🔼)
91/141
🟡 Functions
77.63% (+0.3% 🔼)
118/152
🟡 Lines
79.93% (+1.36% 🔼)
490/613
Show new covered files 🐣
St.
File Statements Branches Functions Lines
🟢
... / SessionNotFoundError.ts
100% 100% 100% 100%

Test suite run success

151 tests passing in 11 suites.

Report generated by 🧪jest coverage report action from dd08eef

Signed-off-by: Michael Kantor <[email protected]>
@kantorcodes kantorcodes changed the title More active session management for stale / removed sessions feat: active session management for stale / removed sessions Nov 26, 2024
Signed-off-by: Michael Kantor <[email protected]>
@kantorcodes kantorcodes marked this pull request as ready for review November 26, 2024 19:33
@kantorcodes kantorcodes requested review from a team as code owners November 26, 2024 19:33
Copy link
Contributor

@justynspooner justynspooner left a comment

Choose a reason for hiding this comment

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

Nice work @kantorcodes

Small query on the session checker logic.

The tests are great to see too. 👍

src/lib/dapp/DAppSigner.ts Outdated Show resolved Hide resolved
Copy link

@itsbrandondev itsbrandondev left a comment

Choose a reason for hiding this comment

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

lgtm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants