Skip to content
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

fix(cmd): useful errors in dag import #9945

Merged
merged 2 commits into from
Jun 14, 2023
Merged

Conversation

lidel
Copy link
Member

@lidel lidel commented Jun 13, 2023

This PR aims to improve UX of importing broken (truncated) CARs.

Most of the time, the error during ipfs dag import is caused by either a bitflip in one of the blocks, or a truncation of the car stream. This PR improves returned error with context that allows the user to understand what happened, and at which place in the car stream, making debug more humane:

$ ipfs dag import ./truncated.car
Error: import failed after block "bafkreieyn4jhc5ht6qmhhqklqi2xvq3mjgn6z5r5mi4ri4hyvpaq3ojvde": unexpected EOF

Also, improved pin error on partial CAR import:

$ curl -v 'http://127.0.0.1:8080/ipns/docs.ipfs.tech?format=car&dag-scope=entity' > ./partial-entity.car # Kubo 0.21.0-rc1 with IPIP-402

Before:

$ ipfs dag import --stats --pin-roots=true ./partial-entity.car
Pinned root	QmPDC11yLAbVw3dX5jMeEuSdk4BiVjSd9X87zaYRdVjzW3	FAILED: block was not found locally (offline): ipld: could not find QmPDvrDAz2aHeLjPVQ4uh1neyknUmDpf1GsBzAbpFhS8ro
Imported 1 blocks (1618 bytes)
[exit code 0] # fake success

After:

$ ipfs dag import --stats --pin-roots=true ./partial-entity.car
Error: pinning root "QmPDC11yLAbVw3dX5jMeEuSdk4BiVjSd9X87zaYRdVjzW3" FAILED: block was not found locally (offline): ipld: could not find QmXZhZq4k5MoZst3YJL8Er8r4qhSjaj2qUGp9Svn9rYb3N
[exit code 1]

Most of the time the error is either a bitflip in one of blocks,
or a truncation of car stream.

This allows user to understand what happened
and at which place in the car stream, making debug more humane.
@lidel lidel requested review from rvagg and aschmahmann June 13, 2023 16:27
@lidel lidel requested a review from a team as a code owner June 13, 2023 16:27
this also correctly exits CLI commands with code 1 (was silent
false-positive 0 before)
@rvagg
Copy link
Member

rvagg commented Jun 14, 2023

seems reasonable, this kind of UX might be nice in some of the go-car standalone cli commands

@lidel lidel marked this pull request as ready for review June 14, 2023 18:45
@lidel
Copy link
Member Author

lidel commented Jun 14, 2023

Ok, merging, as I want to add more tests in #9926 and this will be useful.

@lidel lidel merged commit f5f6b66 into master Jun 14, 2023
@lidel lidel deleted the fix/useful-error-on-dag-import branch June 14, 2023 18:55
@lidel lidel mentioned this pull request Jun 14, 2023
hacdias pushed a commit that referenced this pull request Jun 15, 2023
* fix: useful errors during dag import

Most of the time the error is either a bitflip in one of blocks,
or a truncation of car stream.

This allows user to understand what happened
and at which place in the car stream, making debug more humane.

* fix: correct message when root pin failed

this also correctly exits CLI commands with code 1 (was silent
false-positive 0 before)
hacdias pushed a commit that referenced this pull request Jun 15, 2023
* fix: useful errors during dag import

Most of the time the error is either a bitflip in one of blocks,
or a truncation of car stream.

This allows user to understand what happened
and at which place in the car stream, making debug more humane.

* fix: correct message when root pin failed

this also correctly exits CLI commands with code 1 (was silent
false-positive 0 before)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants