Skip to content

Conversation

@joshenlim
Copy link
Member

@joshenlim joshenlim commented Jun 12, 2025

Resolves FE-1677

Context

We've got a report internally that the storage explorer occasionally shows an error "Failed to retrieve folder contents from ...: bucketId is required". This error stems from fetchFolderContents in the storage explorer store, and fetchFolderContents uses the internal state state.selectedBucket.id to call the request to fetch the contents

StorageExplorer.tsx has a a couple of useEffects - of which two of them have a tendency for a race condition, specifically:

  • The useEffect for openBucket with bucket in the dependency array
  • The useEffect for fetchFolderContents with itemSearchString in the array

Because of this - there's technically a chance of fetchFolderContents being triggered before openBucket was called, in which case state.selectedBucket wouldn't have been set just yet, and hence cause the error

Changes involved

  • fetchFolderContents now accepts the an additional parameter bucketId rather than relying on the internal state of selectedBucket
  • Within the useEffect for fetchFolderContents in the if block for STORAGE_VIEWS.COLUMNS, conditionally call fetchFolderContents if openedFolders.length is 0
  • Added a hook useSelectedBucket so that the currently selected bucket can be accessed easily

To test

  • Mainly just do a brief smoke test of the storage explorer, ensure that everything is behaving as per status quo (open bucket, open folders, etc)

@joshenlim joshenlim requested a review from a team as a code owner June 12, 2025 15:32
@vercel
Copy link

vercel bot commented Jun 12, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
studio-self-hosted ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 16, 2025 6:55am
studio-staging ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 16, 2025 6:55am
6 Skipped Deployments
Name Status Preview Comments Updated (UTC)
cms ⬜️ Ignored (Inspect) Jun 16, 2025 6:55am
studio ⬜️ Ignored (Inspect) Visit Preview Jun 16, 2025 6:55am
design-system ⬜️ Skipped (Inspect) Jun 16, 2025 6:55am
docs ⬜️ Skipped (Inspect) Jun 16, 2025 6:55am
ui-library ⬜️ Skipped (Inspect) Jun 16, 2025 6:55am
zone-www-dot-com ⬜️ Skipped (Inspect) Jun 16, 2025 6:55am

@supabase
Copy link

supabase bot commented Jun 12, 2025

This pull request has been ignored for the connected project xguihxuzqibwxjnimxev because there are no changes detected in supabase directory. You can change this behaviour in Project Integrations Settings ↗︎.


Preview Branches by Supabase.
Learn more about Supabase Branching ↗︎.

Copy link
Member

@alaister alaister left a comment

Choose a reason for hiding this comment

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

LGTM!

  • Tested on preview

@joshenlim joshenlim merged commit 9ded153 into master Jun 17, 2025
21 checks passed
@joshenlim joshenlim deleted the chore/mitigate-race-condition-when-fetching-folder-contents-storage branch June 17, 2025 02:37
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.

2 participants