fix(renterd): first file download no bucket error

[alexfreska]

• Fixed an issue where the first attempt to download a file would show a bucket not found error.

Notes

• Strange issue where buckets.data is still undefined in the closure even though the data has been fetched. Adding useCallback does the trick, oddly even just console logging buckets.data above downloadFiles fixes the issue.

[vercel]

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

Name	Status	Preview	Comments	Updated (UTC)
renterd	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Dec 13, 2024 4:54pm

4 Skipped Deployments

Name	Status	Preview	Comments	Updated (UTC)
explorer	⬜️ Ignored (Inspect)	Visit Preview		Dec 13, 2024 4:54pm
explorer-zen	⬜️ Ignored (Inspect)	Visit Preview		Dec 13, 2024 4:54pm
hostd	⬜️ Ignored (Inspect)	Visit Preview		Dec 13, 2024 4:54pm
website	⬜️ Ignored (Inspect)	Visit Preview		Dec 13, 2024 4:54pm

[changeset-bot]

🦋 Changeset detected

Latest commit: 2bb17b5

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
renterd	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[alexfreska]

• fix(renterd): first file download no bucket error #853 👈 (View in Graphite)
• refactor(renterd): min account expiry and min price table validity units #852
• chore: update cluster #854
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[telestrial]

Strange issue where buckets.data is still undefined in the closure even though the data has been fetched. Adding useCallback does the trick, oddly even just console logging buckets.data above downloadFiles fixes the issue.

I've been mulling this over a bit because it is an interesting issue! On its face, it could appear to treat a symptom and not the cause. That's what I initially thought, but after thinking a bit, here's what I believe is going on:

When you define a function like downloadFiles, it captures the state of buckets.data at that moment. React doesn’t create a new function reference on subsequent renders unless explicitly instructed to (as with useCallback), which is why you're seeing a stale reference. The closure retains the value of buckets.data as it was when the function was first defined, without updating automatically.

The reason console.log has the correct value is that it forces the runtime to evaluate the current value of buckets.data at the time the log statement is executed. So console.log() is the current render and downloadFiles is likely the first render.

All that said, I believe this is exactly the right answer to the problem. It's probably more of a use case for useCallback than most other common uses.

[alexfreska]

@telestrial it has to be something with a stale closure but when downloadFiles is not wrapped in useCallback React does create a new function reference every re-render, for example if I were to try to use it as a dependency for another hook deps you would get a warning like:

'downloadFiles' function makes the dependencies of useEffect Hook (at line 153) change on every render. To fix this, wrap the definition of 'downloadFiles' in its own useCallback() Hook.

The behaviour you describe should be when you do a useCallback with empty deps []. I am really not sure what is causing this - definitely an interesting Advent of Bugs problem haha.

[alexfreska]

Single test failing due to: SiaFoundation/renterd#1747

[alexfreska]

Merge activity

• Dec 13, 11:43 AM EST: A user started a stack merge that includes this pull request via Graphite.
• Dec 13, 11:49 AM EST: Graphite rebased this pull request as part of a merge.
• Dec 13, 11:50 AM EST: A user merged this pull request with Graphite.