Skip to content
This repository has been archived by the owner on Nov 10, 2023. It is now read-only.

fix: improve Safe loading error handling #3728

Merged
merged 8 commits into from
Apr 1, 2022
Merged

Conversation

iamacook
Copy link
Member

What it solves

Safe loading error screen persisting

How this PR fixes it

If the Safe fails to load, but then succeeds on the next scheduled poll, the "Safe couldn't be loaded" error screen is removed. The Safe is only removed from viewed Safes if the user clicks on the error page button.

This is a duplicate of #3717, based off #3727 instead as it includes associated fixes.

How to test it

Open a Safe on a throttled connection and observe the error message appear. When the loading has succeeded, it should now dissapear and not remove it from the viewed Safes cache.

@github-actions
Copy link

CLA Assistant Lite All Contributors have signed the CLA.

@iamacook iamacook changed the base branch from dev to set-web3-middleware March 24, 2022 18:11
@github-actions
Copy link

github-actions bot commented Mar 24, 2022

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 1 0
Ignored 0 N/A
  • Result: ✅ success

  • Annotations: 1 total


[warning] react-hooks/exhaustive-deps

verifies the list of dependencies for Hooks like useEffect and similar


Report generated by eslint-plus-action

@iamacook iamacook self-assigned this Mar 24, 2022
@github-actions
Copy link

Deployment links

🟠 Rinkeby Mainnet 🟣 Polygon 🟡 BSC Arbitrum 🟢 Gnosis Chain

@iamacook iamacook requested a review from katspaugh March 25, 2022 10:49
Base automatically changed from set-web3-middleware to dev March 28, 2022 14:24
Copy link
Member

@usame-algan usame-algan left a comment

Choose a reason for hiding this comment

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

Really clean to move the onboard related logic into one place 👍

src/routes/safe/components/SafeLoadError.tsx Outdated Show resolved Hide resolved
src/routes/safe/container/index.tsx Outdated Show resolved Hide resolved
@coveralls
Copy link

coveralls commented Apr 1, 2022

Pull Request Test Coverage Report for Build 2078045553

  • 6 of 23 (26.09%) changed or added relevant lines in 15 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage increased (+0.02%) to 36.055%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/routes/safe/components/Settings/ManageOwners/AddOwnerModal/screens/OwnerForm/index.tsx 0 1 0.0%
src/routes/safe/components/Settings/ManageOwners/AddOwnerModal/screens/ThresholdForm/index.tsx 0 1 0.0%
src/routes/safe/components/Settings/ManageOwners/RemoveOwnerModal/screens/ThresholdForm/index.tsx 0 1 0.0%
src/routes/safe/components/Settings/ManageOwners/ReplaceOwnerModal/screens/OwnerForm/index.tsx 0 1 0.0%
src/routes/safe/components/Settings/SpendingLimit/NewLimitModal/Review.tsx 0 1 0.0%
src/routes/safe/components/Settings/ThresholdSettings/index.tsx 0 1 0.0%
src/logic/safe/hooks/useSafeScheduledUpdates.tsx 0 2 0.0%
src/routes/safe/components/SafeLoadError.tsx 0 3 0.0%
src/routes/safe/container/index.tsx 0 6 0.0%
Files with Coverage Reduction New Missed Lines %
src/routes/safe/container/index.tsx 1 0%
Totals Coverage Status
Change from base Build 2076321222: 0.02%
Covered Lines: 3517
Relevant Lines: 8868

💛 - Coveralls

@francovenica
Copy link
Contributor

It works, if the safe doesn't load it retries after a few seconds and it loads fine then

I have issues with the store of the "viwed safes". The safe, after failing and then loading properly is not saved in that storage.
In gif I do the whole process, then I reload the page and reload the browser storage and the safe is not in the list.
Only if I visit a 2nd safe then the 1st safe that was used for testing is saved in the storage.
04-01-2022_x(4609)

@francovenica
Copy link
Contributor

Awsome. cool fix.

Looks good to me

@iamacook iamacook merged commit 88080c8 into dev Apr 1, 2022
@iamacook iamacook deleted the improve-safe-load-error branch April 1, 2022 15:47
@github-actions github-actions bot locked and limited conversation to collaborators Apr 1, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants