PE-9205: Login reads walk the gateway waterfall they were meant to skip - #2182
Merged
Merged
Conversation
A drive row whose rootFolderId has no matching folder row cannot be opened at all. watchFolderContents drops the missing folder with a `.where`, so its combined stream never emits and the explorer is stranded on a spinner that nothing retries. It happens when the root folder's own metadata fails to resolve during sync while the drive entity resolves fine - more reachable since sync moved from a four gateway waterfall to two attempts on one. - write a root folder placeholder alongside every drive, in updateUserDrives and writeDriveEntity, mirroring what createDrive already does. rootFolderId is known from the drive entity either way, so the row can always be written - insert with InsertMode.insertOrIgnore so it never overwrites a real root folder. re-running on every sync also heals drives already in this state, and recovery is live: the Drift stream is already watching the row - the placeholder is deliberately not marked isGhost. toEntryCompanion omits that column and the upsert landing real metadata leaves absent columns untouched, so the flag would stick forever. parentFolderId is null rather than self referencing, which is why createGhosts excludes root folders and why this is a placeholder rather than a ghost - _handleFolderNotFound now emits DriveDetailLoadNotFound for a genuinely absent drive and DriveDetailLoadUnsynced otherwise. Both are re-checked by _onSyncCompleted; DriveInitialLoading was a dead end with no retry and nothing that re-triggers it - fold openFolder's duplicate inline onError handler into _handleFolderNotFound No schema change: parentFolderId is nullable and isGhost defaults false. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DnYLXFocWgTt9M2CbGYSUP
…9205 Both from CodeRabbit review on #2181. - _handleFolderNotFound checked only isClosed after awaiting the drive query, so a drive switched during that await could emit the previous drive's state onto the new drive's screen. Guard on _driveId, matching the check the success path already does in the same subscription. This mattered more after the previous commit: the emit now carries the stale drive object rather than a generic loading state - writeDriveEntity wrote the drive and its root folder as two separate inserts, leaving a window where an observer could see the drive-without-a-root-folder state the placeholder exists to rule out. One transaction, matching insertDriveRevision and friends. updateUserDrives already got this from db.batch Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DnYLXFocWgTt9M2CbGYSUP
An empty-looking drive is ambiguous: genuinely empty, or never synced. Telling someone their drive is empty when we have not actually read it reads as "my data is gone". Making the root folder row always exist made this the common case rather than a rare one, because the drive now opens instead of failing, and DriveDetailLoadUnsynced - the existing "Drive Not Synced" screen with its sync action - was only ever reached through the missing-root-folder error that fix removes. Restores that screen from an honest signal instead of an exception: when a drive renders empty at its root folder and no revision exists for that root folder, we have never seen its metadata, so emit DriveDetailLoadUnsynced rather than claiming the drive is empty. The root folder revision is the right signal. A drive created in-app writes one at creation (DriveCreateCubit), and sync writes one when real metadata lands, but a drive discovered by updateUserDrives has only the placeholder row until then. lastBlockHeight cannot answer this: the column defaults to 0, so a freshly created empty drive is indistinguishable from one that has never synced - gating on it would show "Drive Not Synced" to someone who just made a drive. Partial syncs are unchanged: any content at all still renders. Reuses existing copy, so no .arb changes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DnYLXFocWgTt9M2CbGYSUP
Both from CodeRabbit review on #2181. - DriveDetailLoadUnsynced is now entered when the root folder has no revision, but all three paths out of it still tested lastBlockHeight. Asymmetric conditions strand the state, and this is the codebase where they come apart: a sync that writes the root revision then fails before advancing the watermark leaves readable metadata behind a drive pinned on "Drive Not Synced", where the sync button only re-emits it. Added _hasRootFolderMetadata and used it at all three sites. lastBlockHeight is still honoured, since a drive synced by an earlier build is synced by definition - cancelling _folderSubscription does not cancel a callback that already began awaiting, so an in-flight load could emit over a newer folder in the same drive, where the _driveId checks cannot see it. Added a _folderLoadGeneration claimed before openFolder's first await and checked after every async boundary that precedes an emit Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DnYLXFocWgTt9M2CbGYSUP
Three of the four defects found on this branch were async timing or state-machine consistency problems in this cubit, and it had no tests at all. The harness runs a real in-memory database and a real DriveDao and mocks only what sits outside the drive explorer, because every one of those defects lived in how the cubit reacts to what the database streams actually emit. A stubbed DriveDao reproduces none of them. Covers: an unsynced drive is not reported as empty; a locally created empty drive is not reported as unsynced; a drive with contents renders; a drive with an advanced watermark opens; and pressing "Sync now" does not return to the unsynced screen once the root metadata is readable. That last test asserts on the emission sequence rather than the final state, which is the only thing that works here. The folder subscription stays live and writing the revision touches tables it watches, so the stream re-fires and repairs the state regardless of what the direct path emitted - an end-state assertion passes even with the bug present. Both recovery tests were mutation checked against the pre-fix condition; only the sequence one fails, and the weaker test says so in its comment rather than claiming cover it does not have. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DnYLXFocWgTt9M2CbGYSUP
…-9205 PE-9203 moved sync onto the configured gateway alone, but classified the read by who asked rather than by how it behaves. getLatestDriveEntityWithId was filed under user-initiated, so login went down the full waterfall: primary once, up to 2 GAR gateways, then arweave.net - plus a Solana RPC for the GAR list, on the startup path PE-9203 exists to keep clear of. - getLatestDriveEntityWithId takes configuredGatewayOnly, and the login path in _validateUser passes it. Attaching a drive by id is untouched: that is one read a user is waiting on and can retry, where breadth is worth its cost - the waterfall's configured gateway now gets one retry, and only on a 404, matching what _syncFetch already does for the same reason. A gateway mid-index answers 404 then 200 a moment later, and leaving on the first 404 is worst for the data most likely to be behind: something just uploaded through Turbo can be on the configured gateway and not yet anywhere else, so falling through reaches gateways further behind it, not ahead. Not extended to timeouts or socket errors, which have already spent their timeout and say the gateway is unwell Both new tests were mutation checked against a single-attempt primary. Auth test stubs gained the new named argument, without which mocktail stops matching the call and returns null. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DnYLXFocWgTt9M2CbGYSUP
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Visit the preview URL for this PR (updated for commit 571e2ed): https://ardrive-web--pr2182-pe-9205-gateway-fall-unub7mop.web.app (expires Wed, 19 Aug 2026 03:32:31 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: a224ebaee2f0939e7665e7630e7d3d6cd7d0f8b0 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What was happening
Logging in with a wallet holding a private drive: the drive entity read hits the configured gateway, gets a 404, and immediately moves to
arweave.net— never retrying the configured gateway.Why
PE-9203 moved sync onto the configured gateway alone. But the split was made by who asked rather than how the read behaves:
fetchDataForSync— configured gateway, 2 attempts, no GAR, no Solana RPCfetchData— primary → up to 2 GAR →arweave.net, each tried oncegetLatestDriveEntityWithIdwas filed under "single user-initiated operations", so login took the waterfall. But login is bulk, at startup, with the user watching a spinner — exactly the profile PE-9203 was written to protect. It also asksArioSDK.getGateways(), costing a Solana RPC on the startup path.That also explains why there was no GAR hop in between:
_buildClientListcaches an empty list when the RPC fails or exceeds its 5s timeout, collapsing the waterfall toprimary → arweave.net.Changes
1. Classify the read by behavior.
getLatestDriveEntityWithIdtakesconfiguredGatewayOnly, and_validateUser(login) passes it. Attaching a drive by id is deliberately untouched — that's one read a user is waiting on and can retry, where breadth is worth its cost. This mirrors theforSyncsplit already in_getDriveSignature.2. Give the waterfall's configured gateway one retry, on 404 only.
_syncFetchalready does this, with reasoning that applies at least as well here:Leaving on the first 404 is worst for the data most likely to be behind: something just uploaded through Turbo can be on the configured gateway and not yet anywhere else, so falling through reaches gateways further behind it, not ahead. Deliberately not extended to timeouts or socket errors — those have already spent their 5s and say the gateway is unwell rather than a beat behind, so the next gateway is the better bet and the 25s total budget stays intact.
Verification
flutter analyzeclean; 1337 tests passNot included
_buildClientListstill caches an empty GAR list for the whole session, so one slow Solana RPC silently disables GAR fallback for every later read. Real, but a separate change — happy to do it next.🤖 Generated with Claude Code
https://claude.ai/code/session_01DnYLXFocWgTt9M2CbGYSUP