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

BLO-835 fix: activity errors #1822

Merged
merged 2 commits into from
Feb 24, 2023
Merged

BLO-835 fix: activity errors #1822

merged 2 commits into from
Feb 24, 2023

Conversation

simonheys
Copy link
Collaborator

Issue / feature description

'Activity' screen not resilient to unexpected conditions of network / backend / individual tx parsing

Changes

  • refactor to remove suspense for more granular error handling, ignore network failures
  • error boundary on each individual row for localised parse errors
  • improve screen error boundary to allow reporting

Checklist

  • Rebased to the last commit of the target branch (or merged)
  • Code self-reviewed
  • Code self-tested
  • Tests updated (if needed)
  • All tests are passing locally

return (
<>
{Object.entries(activity).map(([dateLabel, transactions]) => (
<Fragment key={dateLabel}>
<HeaderCell>{dateLabel}</HeaderCell>
{transactions.map((transaction) => {
if (isActivityTransaction(transaction)) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This was refactored into AccountActivityItem so it can be more easily wrapped in an error boundary

return (
<CellStack>
<CellStack key={accountIdentifier} flex={1}>
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

key to ensure the component re-renders on network change

@simonheys simonheys marked this pull request as ready for review February 23, 2023 15:16
@github-actions
Copy link

Builds for local testing

@@ -146,6 +146,7 @@ export const useArgentExplorerAccountTransactionsInfinite = (
)
return useSWRInfinite<IExplorerTransaction[]>(key, argentApiFetcher, {
revalidateAll: true,
shouldRetryOnError: false /** expect errors on unsupported networks */,
Copy link
Contributor

Choose a reason for hiding this comment

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

isnt this risky ? Like I understand that we should expect errors for some networks but retryOnError allows to course correct if the backend is restarting for example

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Risk both ways - at the moment we see frequent 500 errors which lead to throttling if the extension keeps retrying. If there are errors then the user will see fallback local tx anyway.

@sonarcloud
Copy link

sonarcloud bot commented Feb 24, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@simonheys simonheys merged commit bed1025 into develop Feb 24, 2023
@simonheys simonheys deleted the fix/BLO-835-activity-errors branch February 24, 2023 13:56
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