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 userAssets query enabled config #6243

Merged
merged 14 commits into from
Nov 1, 2024
Merged

Fix userAssets query enabled config #6243

merged 14 commits into from
Nov 1, 2024

Conversation

jinchung
Copy link
Member

@jinchung jinchung commented Oct 29, 2024

Fixes APP-1977

What changed (plus any additional context for devs)

There is a race condition that can happen when we first start the app and do a user assets query function call without the address. We weren't disabling the query function call, resulting in us to just return an empty state and therefore empty objects. There could be a race condition from when we set the incorrect data but before we get the latest data with the account address, when a user could open the swap state, preventing further updates from the user assets store from getting synced.

There were a few different issues happening at the same time, but the retry errors we were getting were not causing this - they were other issues.

This will also be improved once we have Ben's user assets consolidation work in place - we should just make sure to keep some of the changes we introduced here, some of which is more aligned with the original user assets query logic.

Screen recordings / screenshots

RPReplay_Final1730337157.MP4

What to test

Try to open swaps right after starting the app - the input asset may not be selected by default automatically if it hasn't loaded in time, but the input asset list should refresh with updated assets when they arrive.

Copy link

linear bot commented Oct 29, 2024

@jinchung jinchung changed the title Fix userAssets query enabled config WIP Fix userAssets query enabled config Oct 29, 2024
@jinchung jinchung force-pushed the @jin/user-assets-retry branch from 4c55bd1 to 785117c Compare October 29, 2024 18:35
try {
const url = `/${chainId}/${address}/assets/?currency=${currency.toLowerCase()}`;
Copy link
Member Author

Choose a reason for hiding this comment

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

this was where we were seeing the "No auth" error message on the retries - have it following the same pattern we have elsewhere now

@jinchung jinchung changed the title WIP Fix userAssets query enabled config Fix userAssets query enabled config Oct 31, 2024
@jinchung jinchung requested review from benisgold and walmat October 31, 2024 01:07
@jinchung jinchung force-pushed the @jin/user-assets-retry branch from 600978a to f3f7994 Compare October 31, 2024 01:35
Copy link
Contributor

@walmat walmat left a comment

Choose a reason for hiding this comment

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

Looks good. I think after fixing the serialization issues, this should be good to merge

@jinchung jinchung force-pushed the @jin/user-assets-retry branch from 1415171 to 0e8830c Compare October 31, 2024 17:10
@jinchung jinchung requested a review from walmat October 31, 2024 17:24
@brunobar79 brunobar79 added the release for release blockers and release candidate branches label Oct 31, 2024
@walmat walmat merged commit cb2b8a0 into develop Nov 1, 2024
7 of 8 checks passed
@walmat walmat deleted the @jin/user-assets-retry branch November 1, 2024 14:32
Copy link

sentry-io bot commented Nov 2, 2024

Suspect Issues

This pull request was deployed and Sentry observed the following issues:

  • ‼️ React ErrorBoundary TypeError: undefined is not a function serializeUserAssetsState(src/state/assets/userA... View Issue
  • ‼️ React ErrorBoundary TypeError: undefined is not a function serializeUserAssetsState(src/state/assets/userA... View Issue
  • ‼️ React ErrorBoundary TypeError: undefined is not a function serializeUserAssetsState(src/state/assets/userA... View Issue

Did you find this useful? React with a 👍 or 👎

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

Successfully merging this pull request may close these issues.

4 participants