Skip to content

fix ipfs fetch#520

Closed
victhorbi wants to merge 1 commit into
mainfrom
fix-ipfs-fetch
Closed

fix ipfs fetch#520
victhorbi wants to merge 1 commit into
mainfrom
fix-ipfs-fetch

Conversation

@victhorbi
Copy link
Copy Markdown
Collaborator

There are cases in which the frontend is passing a wrong CID

There are cases in which the frontend is passing a wrong CID
@sonarqubecloud
Copy link
Copy Markdown

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR aims to prevent IPFS proposal-detail fetching from breaking when the frontend supplies an incorrect/absent CID, by skipping IPFS fetches when the CID field is falsy.

Changes:

  • Skip getProposalsFromIpfs calls when ipfsHash/description is falsy in proposal enrichment flows.
  • Update README Node.js requirement to v22+.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.

File Description
apps/frontend/src/utils/proposals/proposalQueriesUtils.ts Adds a guard to avoid IPFS fetch when ipfsHash is falsy during proposal enrichment.
apps/frontend/src/utils/proposals/helpers.ts Adds a guard to avoid IPFS fetch when historical proposal description (CID) is falsy.
README.md Updates documented Node.js requirement to v22+.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

const ipfsFetches = proposals.map(p =>
p.ipfsHash ? getProposalsFromIpfs(p.ipfsHash) : Promise.resolve({ ipfsHash: undefined }),
);
const ipfsDetails: IpfsDetails[] = await Promise.all(ipfsFetches);
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

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

Promise.all(ipfsFetches) will reject if any getProposalsFromIpfs call fails (e.g., 404/timeout for an invalid CID), which can break the whole proposals load. Consider making each fetch resilient (wrap each call with .catch(() => ({})) / return a fallback IpfsDetails, or use Promise.allSettled and merge only fulfilled results).

Suggested change
const ipfsDetails: IpfsDetails[] = await Promise.all(ipfsFetches);
const ipfsDetailsResults = await Promise.allSettled(ipfsFetches);
const ipfsDetails: IpfsDetails[] = ipfsDetailsResults.map(result =>
result.status === "fulfilled" ? result.value : ({ ipfsHash: undefined } as IpfsDetails),
);

Copilot uses AI. Check for mistakes.
Comment on lines +19 to +22
// Only fetch from IPFS if ipfsHash is valid
const ipfsFetches = proposals.map(p =>
p.ipfsHash ? getProposalsFromIpfs(p.ipfsHash) : Promise.resolve({ ipfsHash: undefined }),
);
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

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

This presence-guard logic is now duplicated at call sites. A more maintainable approach is to have getProposalsFromIpfs short-circuit when the CID is missing/empty and return an empty IpfsDetails, so callers can simply map getProposalsFromIpfs(p.ipfsHash) without inline Promise.resolve(...) fallbacks.

Suggested change
// Only fetch from IPFS if ipfsHash is valid
const ipfsFetches = proposals.map(p =>
p.ipfsHash ? getProposalsFromIpfs(p.ipfsHash) : Promise.resolve({ ipfsHash: undefined }),
);
const ipfsFetches = proposals.map(p => getProposalsFromIpfs(p.ipfsHash));

Copilot uses AI. Check for mistakes.
if (!data) return [];
const ipfsFetches = data.map(d => getProposalsFromIpfs(d.description));

// Only fetch from IPFS if description (CID) is valid
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

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

The comment says the description/CID is "valid", but this only checks truthiness. If you intend to prevent bad CIDs, either validate/normalize the CID (e.g., trim, strip ipfs:// if applicable, or add a real CID validator) or adjust the comment to match the actual behavior.

Suggested change
// Only fetch from IPFS if description (CID) is valid
// Only fetch from IPFS if a description (CID) is present

Copilot uses AI. Check for mistakes.
Comment on lines +249 to 251
d.description ? getProposalsFromIpfs(d.description) : Promise.resolve({ ipfsHash: undefined }),
);
const ipfsDetails = await Promise.all(ipfsFetches);
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

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

As written, a single failing IPFS request will reject Promise.all(ipfsFetches) and prevent parsing all historical proposals. To make this robust to invalid/missing CIDs or transient IPFS errors, consider catching per-item failures (or using Promise.allSettled) and falling back to empty IpfsDetails for that proposal.

Suggested change
d.description ? getProposalsFromIpfs(d.description) : Promise.resolve({ ipfsHash: undefined }),
);
const ipfsDetails = await Promise.all(ipfsFetches);
d.description ? getProposalsFromIpfs(d.description) : Promise.resolve({ ipfsHash: undefined } as IpfsDetails),
);
const ipfsResults = await Promise.allSettled(ipfsFetches);
const ipfsDetails = ipfsResults.map(result =>
result.status === "fulfilled" ? result.value : ({ ipfsHash: undefined } as IpfsDetails),
);

Copilot uses AI. Check for mistakes.
Comment thread README.md
Ensure your development environment is set up with the following:

- **Node.js (v18 or later):** [Download here](https://nodejs.org/en/download/package-manager) 📥
- **Node.js (v22 or later):** [Download here](https://nodejs.org/en/download/package-manager) 📥
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

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

README now states Node.js v22+, but the repo root package.json still declares engines.node as >=18. Please align these (either bump engines to >=22 if v22 is required, or keep README at v18+ if older Node versions are supported).

Suggested change
- **Node.js (v22 or later):** [Download here](https://nodejs.org/en/download/package-manager) 📥
- **Node.js (v18 or later):** [Download here](https://nodejs.org/en/download/package-manager) 📥

Copilot uses AI. Check for mistakes.
Comment on lines +19 to +22
// Only fetch from IPFS if ipfsHash is valid
const ipfsFetches = proposals.map(p =>
p.ipfsHash ? getProposalsFromIpfs(p.ipfsHash) : Promise.resolve({ ipfsHash: undefined }),
);
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

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

The comment says the hash is "valid", but the condition only checks truthiness (non-empty). Either validate the CID format (or trim whitespace before checking), or reword the comment to reflect that this is only a presence check.

Suggested change
// Only fetch from IPFS if ipfsHash is valid
const ipfsFetches = proposals.map(p =>
p.ipfsHash ? getProposalsFromIpfs(p.ipfsHash) : Promise.resolve({ ipfsHash: undefined }),
);
// Only fetch from IPFS if a non-empty ipfsHash is present
const ipfsFetches = proposals.map(p => {
const ipfsHash = p.ipfsHash?.trim();
return ipfsHash ? getProposalsFromIpfs(ipfsHash) : Promise.resolve({ ipfsHash: undefined });
});

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants