Skip to content

fix(storage): trim provider resolver & pass all erorrs through#702

Merged
hugomrdias merged 1 commit intomasterfrom
rvagg/clean-enrichment
Mar 30, 2026
Merged

fix(storage): trim provider resolver & pass all erorrs through#702
hugomrdias merged 1 commit intomasterfrom
rvagg/clean-enrichment

Conversation

@rvagg
Copy link
Copy Markdown
Collaborator

@rvagg rvagg commented Mar 30, 2026

validateDataSet is mostly redundant here because getClientDataSets already applies roughly the same filters so it's primarily acting as an invariant (i.e. does PDPVerifier agree that this is still active and managed by FWSS?). So we can remove one RPC call per data set and it's the call that can throw, so all other errors that come from these RPC calls are going to be problems for the user to deal with (such as RPC rate limit errors).

validateDataSet throwing here is one of those "throw == false" situations, but of course we're not doing any sophisticated checking here and swalling any other kind of error, including RPC. The theory behind not being strict is something like: it's not critical that we find a matching data set, it's more critical that we get the user's data uploaded. But in the case of dealbot which is trying to be precise, that falls apart and our user in that case doesn't want any laxity.

validateDataSet is mostly redundant here because getClientDataSets already
applies roughly the same filters so it's primarily acting as an invariant
(i.e. does PDPVerifier agree that this is still active and managed by FWSS?).
So we can remove one RPC call per data set and it's the call that can throw,
so all other errors that come from these RPC calls are going to be problems
for the user to deal with (such as RPC rate limit errors).
@rvagg rvagg requested a review from juliangruber March 30, 2026 03:11
@rvagg rvagg requested a review from hugomrdias as a code owner March 30, 2026 03:11
@github-project-automation github-project-automation bot moved this to 📌 Triage in FOC Mar 30, 2026
@rvagg
Copy link
Copy Markdown
Collaborator Author

rvagg commented Mar 30, 2026

The other instances of validateDataSet in here are still reasonable, one is resovleByDataSetId and we do want to check that it's both active and managed by FWSS, and the other is in commit() which is acting like a final "this is going to work, right?", in both of those cases the error will be returned.

@cloudflare-workers-and-pages
Copy link
Copy Markdown

cloudflare-workers-and-pages bot commented Mar 30, 2026

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Updated (UTC)
❌ Deployment failed
View logs
synapse-dev 6fe56be Mar 30 2026, 03:28 AM

@github-project-automation github-project-automation bot moved this from 📌 Triage to ✔️ Approved by reviewer in FOC Mar 30, 2026
@rjan90 rjan90 added this to the M4.2: mainnet GA milestone Mar 30, 2026
@hugomrdias hugomrdias merged commit 02a0fc6 into master Mar 30, 2026
13 of 14 checks passed
@hugomrdias hugomrdias deleted the rvagg/clean-enrichment branch March 30, 2026 11:23
@github-project-automation github-project-automation bot moved this from ✔️ Approved by reviewer to 🎉 Done in FOC Mar 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: 🎉 Done

Development

Successfully merging this pull request may close these issues.

3 participants