-
Notifications
You must be signed in to change notification settings - Fork 44
ask for child blocks and retry on errors fetching data #2243
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Conversation
Marketen
commented
Sep 5, 2025
- retry if something goes wrong when fetching CAR up to 3 times
- ask for whole DAG instead of only the root CID (should fix "could not get block" errors)
There was a problem hiding this 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 enhances the IPFS data fetching mechanism by implementing retry logic and requesting complete DAG structures instead of just root CIDs to improve reliability.
- Adds retry logic with up to 3 attempts when fetching CAR data from IPFS
- Changes the request to fetch entire DAG structures using
dag-scope=all&order=dfsparameters - Introduces debug logging to track block counts and child block presence
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| let blockCount = 0; | ||
| let hasChildren = false; | ||
| for await (const { cid } of carReader.blocks()) { | ||
| blockCount++; | ||
| if (!cid.equals(root)) hasChildren = true; | ||
| } | ||
| console.debug( | ||
| `[IPFS] CAR stats: blocks=${blockCount} hasChildren=${hasChildren}` | ||
| ); |
Copilot
AI
Sep 5, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The block iteration for logging purposes adds unnecessary overhead on every successful request. Consider moving this debug logging behind a debug flag or removing it entirely, as it iterates through all blocks just for logging.
| let blockCount = 0; | |
| let hasChildren = false; | |
| for await (const { cid } of carReader.blocks()) { | |
| blockCount++; | |
| if (!cid.equals(root)) hasChildren = true; | |
| } | |
| console.debug( | |
| `[IPFS] CAR stats: blocks=${blockCount} hasChildren=${hasChildren}` | |
| ); | |
| if (process.env.DEBUG) { | |
| let blockCount = 0; | |
| let hasChildren = false; | |
| for await (const { cid } of carReader.blocks()) { | |
| blockCount++; | |
| if (!cid.equals(root)) hasChildren = true; | |
| } | |
| console.debug( | |
| `[IPFS] CAR stats: blocks=${blockCount} hasChildren=${hasChildren}` | |
| ); | |
| } |
| } catch (e) { | ||
| lastError = e; | ||
| // Wait a bit before retrying | ||
| if (attempt < 2) await new Promise(r => setTimeout(r, 500)); |
Copilot
AI
Sep 5, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The magic number 500 (milliseconds) should be defined as a named constant to improve readability and maintainability.
|
Dappnode bot has built and pinned the built packages to an IPFS node, for commit: d9c2755 This is a development version and should only be installed for testing purposes.
Hash: (by dappnodebot/build-action) |