Skip to content
This repository was archived by the owner on Oct 4, 2023. It is now read-only.

[C-1764] Fix offline app initialization for offline mode#2560

Merged
amendelsohn merged 8 commits into
mainfrom
amendel-offline-lineups-with-dylan
Jan 10, 2023
Merged

[C-1764] Fix offline app initialization for offline mode#2560
amendelsohn merged 8 commits into
mainfrom
amendel-offline-lineups-with-dylan

Conversation

@amendelsohn

Copy link
Copy Markdown
Contributor

Description

An error thrown in the wallet saga fetchBalanceAsync were causing all sagas to stop. After which point, no new actions were being processed.

FetchBalanceAsync should have been waiting for audiusBackendInstance to be setup, but was not.

We've split the reachability check into more states, adding an 'unconfirmed' state which can be used to optimistically assume reachability unless it is explicitly false. We use this for read calls that can be made without the full backend and will simply fail to respond. For writes requiring audiusBackend setup, we will use the more strict check that will only pass if reachability has been tested and confirmed true.

Changes:

  1. Added third 'unconfirmed' reachability state
  2. Added error logging to saga onError handler
  3. Optimistically ApiClient.init() and setup backend
  4. Retry setupBackend on every change to reachability if it hasn't been successfully set up yet
  5. WaitForWrite now waits for confirmed reachability

Dragons

Changes to app startup are always scary.

WaitForRead and WaitForWrite are intuitive, but do not necessarily cover all the cases. We may want to take another pass at those soon.

How Has This Been Tested?

iOS Simulator

TODO:

  • make sure we didn't break web

How will this change be monitored?

For features that are critical or could fail silently please describe the monitoring/alerting being added.

Feature Flags

This is part of offline mode work, but notably not covered by the feature flag.

@amendelsohn amendelsohn changed the title Fix offline app initialization for offline mode [C-1764] Fix offline app initialization for offline mode Jan 10, 2023
const sagaMiddleware = createSagaMiddleware({
context: storeContext,
onError: (e) => {
console.error('Caught Saga Error', e, e.stack)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

we will get this in sentry now too. this is great!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sweet!! Is this a reasonable way to log it for readability in sentry?
I just wrote this for local debugging, but figured it's good to include

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this should be enough. they'll truncate our stack trace, but we'll learn quickly

export function* waitForWrite() {
yield* call(waitForBackendSetup)
yield* call(waitForAccount)
yield* call(waitForConfirmedReachability)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

seems reasonable for now. agreed to discuss at client sync.


function* fetchBalanceAsync() {
yield* waitForRead()
yield* waitForWrite()

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

maybe a comment here to explain why since this is unintuitivie

@sliptype sliptype left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Makes sense to me, let's def discuss the WaitForRead/Write stuff!

Comment thread packages/web/src/store/reachability/sagas.ts Outdated
Co-authored-by: Sebastian Klingler <sliptype@gmail.com>

@dylanjeffers dylanjeffers left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

lgtm!

yield* put(accountActions.fetchLocalAccount())

// Init APICLient
const apiClient = yield* getContext('apiClient')

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

do we need to move this now that we are technically just going through with init?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think the change is not required, but it works this way too

@audius-infra

Copy link
Copy Markdown
Collaborator

@amendelsohn amendelsohn merged commit 15b26fd into main Jan 10, 2023
@amendelsohn amendelsohn deleted the amendel-offline-lineups-with-dylan branch January 10, 2023 23:49
audius-infra pushed a commit that referenced this pull request Jan 14, 2023
[646a2ee] [C-1772] Fix share-drawer for tracks with missing images (#2594) Dylan Jeffers
[ea615c6] [C-1829] Fix android build due to gradle dep (#2592) Dylan Jeffers
[708563a] [C-1778] Fix update banner styles (#2591) Dylan Jeffers
[aa51116] update playlist_page route to accept permalink optionally (#2569) sabrina-kiam
[8ed8846] Move stream mp3 feature flag check later so optimizely is ready (#2590) Andrew Mendelsohn
[8c19e15] [PAY-778] Update track page for premium tracks (#2565) Saliou Diallo
[9bc901e] [C-1643] Update track player to work with the queue in the redux store (#2580) Kyle Shanks
[6e9c2d1] Make rc-staging fast-app variant (#2585) Dylan Jeffers
[c75163a] Fix notification userInteraction (#2583) Dylan Jeffers
[2d9a92f] [PAY-817] Add premium content to track upload flow on desktop - Part 2 (#2546) Saliou Diallo
[e9b0673] [C-1821] Fix android manual sign up overflow (#2582) Sebastian Klingler
[c26a6c4] Add null check for notification navigation hook (#2581) Kyle Shanks
[b62a866] Entity manager notification writes (#2556) Joseph Lee
[b685c89] [C-1819] Fix customCheck in waitForValue (#2578) Sebastian Klingler
[4ddbd37] Bump ios to 1.1.52 (#2577) Sebastian Klingler
[70a9fd5] [C-1816] Fix divider color (#2574) Sebastian Klingler
[0796f47] Hotfix: Fix mobile build after selector was changed (#2575) Marcus Pasell
[0d513a7] DMs UI Part 1 (#2563) Marcus Pasell
[0e24bbc] [C-1726] useLocalImage (#2564) Sebastian Klingler
[6329be8] [C-1815] Fix legacy image loading (#2573) Sebastian Klingler
[57d72ab] [C-1814] Prevent infinite rerender of UserList (#2572) Sebastian Klingler
[b3b00e8] [C-1706] Fix sidebar track statistic (#2570) Dylan Jeffers
[abb17ef] Fix setup states (#2567) Raymond Jacobson
[c9f158e] Remove EntityManager gating for users and tracks (#2549) Isaac Solo
[adea3ff] [C-1760] Fix lineup-tile animation lag (#2548) Dylan Jeffers
[3c3426d] [C-1791] Add fast-cache, safe-fast-cache (#2554) Dylan Jeffers
[15b26fd] [C-1764] Fix offline app initialization for offline mode (#2560) Andrew Mendelsohn
[bd9ab4d] [C-1790] Improve lottie colorize perf (#2552) Dylan Jeffers
[120cce4] [C-1397] Add appVersion to optimizely audience attributes (#2561) Sebastian Klingler
[32962ec] Bump ios to 1.1.51 (#2558) Sebastian Klingler
[bd40d44] [QA-282] Fix app crash on edit playlist screen when missing collection image source (#2559) Kyle Shanks
[ee0e3cd] Update the feature flag for rate cta drawer (#2557) Kyle Shanks
[c0c12d7] Add back arm64 exclusion (#2531) Marcus Pasell
[7287c4c] [PAY-803] [PAY-766] Initial DMs client integration (#2521) Marcus Pasell
[effd347] [PAY-367] [C-1786 C-1661] Refactor FeedTipTile/Lineup interdependency (#2547) Dylan Jeffers
[6eb31a8] [PAY-808] [PAY-810] Float new playlists to top of library (#2526) Michael Piazza
[9c133b5] [C-1789] Fix lottie colorize crash (#2551) Sebastian Klingler
[a27b24d] [C-1418] Add new android bundles (#2523) Sebastian Klingler
[12dd5e8] [C-1788][C-1784][C-1785] Fix offline queue loops and duplicates (#2550) Andrew Mendelsohn
[fc4c8d2] [C-1779 Improve makeStyles performance (#2541) Dylan Jeffers
[08b79bc] [C-1766][C-1781] Fix loading states on DownloadToggle (#2545) Andrew Mendelsohn
[470a44f] [C-1756][C-1767] Fix job cancellation for track download queue (#2543) Andrew Mendelsohn
[bd15357] [C-1703] fix button padding for remove download playlist (#2544) Andrew Mendelsohn
[6eb6b74] [PAY-776] Add premium content to track upload flow on desktop - Part 1 (#2469) Saliou Diallo
@AudiusProject AudiusProject deleted a comment from linear Bot Sep 11, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants