Merge pull request #8838 from Jorropo/fix/ErrNotFound-hopefully-final

chore: bump go-ipld-format v0.4.0 and fix related sharness tests

blocks/blockstoreutil/remove.go

@@ -3,8 +3,8 @@ package blockstoreutil
 
 import (
 	"context"
+	"errors"
 	"fmt"
-	"io"
 
 	cid "github.com/ipfs/go-cid"
 	bs "github.com/ipfs/go-ipfs-blockstore"
@@ -13,14 +13,14 @@ import (
 )
 
 // RemovedBlock is used to represent the result of removing a block.
-// If a block was removed successfully, then the Error string will be
-// empty.  If a block could not be removed, then Error will contain the
+// If a block was removed successfully, then the Error will be empty.
+// If a block could not be removed, then Error will contain the
 // reason the block could not be removed.  If the removal was aborted
 // due to a fatal error, Hash will be empty, Error will contain the
 // reason, and no more results will be sent.
 type RemovedBlock struct {
-	Hash  string `json:",omitempty"`
-	Error string `json:",omitempty"`
+	Hash  string
+	Error error
 }
 
 // RmBlocksOpts is used to wrap options for RmBlocks().
@@ -51,17 +51,17 @@ func RmBlocks(ctx context.Context, blocks bs.GCBlockstore, pins pin.Pinner, cids
 			// remove this sometime in the future.
 			has, err := blocks.Has(ctx, c)
 			if err != nil {
-				out <- &RemovedBlock{Hash: c.String(), Error: err.Error()}
+				out <- &RemovedBlock{Hash: c.String(), Error: err}
 				continue
 			}
 			if !has && !opts.Force {
-				out <- &RemovedBlock{Hash: c.String(), Error: format.ErrNotFound{Cid: c}.Error()}
+				out <- &RemovedBlock{Hash: c.String(), Error: format.ErrNotFound{Cid: c}}
 				continue
 			}
 
 			err = blocks.DeleteBlock(ctx, c)
 			if err != nil {
-				out <- &RemovedBlock{Hash: c.String(), Error: err.Error()}
+				out <- &RemovedBlock{Hash: c.String(), Error: err}
 			} else if !opts.Quiet {
 				out <- &RemovedBlock{Hash: c.String()}
 			}
@@ -79,7 +79,7 @@ func FilterPinned(ctx context.Context, pins pin.Pinner, out chan<- interface{},
 	stillOkay := make([]cid.Cid, 0, len(cids))
 	res, err := pins.CheckIfPinned(ctx, cids...)
 	if err != nil {
-		out <- &RemovedBlock{Error: fmt.Sprintf("pin check failed: %s", err)}
+		out <- &RemovedBlock{Error: fmt.Errorf("pin check failed: %w", err)}
 		return nil
 	}
 	for _, r := range res {
@@ -88,36 +88,9 @@ func FilterPinned(ctx context.Context, pins pin.Pinner, out chan<- interface{},
 		} else {
 			out <- &RemovedBlock{
 				Hash:  r.Key.String(),
-				Error: r.String(),
+				Error: errors.New(r.String()),
 			}
 		}
 	}
 	return stillOkay
 }
-
-// ProcRmOutput takes a function which returns a result from RmBlocks or EOF if there is no input.
-// It then writes to stdout/stderr according to the RemovedBlock object returned from the function.
-func ProcRmOutput(next func() (interface{}, error), sout io.Writer, serr io.Writer) error {
-	someFailed := false
-	for {
-		res, err := next()
-		if err == io.EOF {
-			break
-		} else if err != nil {
-			return err
-		}
-		r := res.(*RemovedBlock)
-		if r.Hash == "" && r.Error != "" {
-			return fmt.Errorf("aborted: %s", r.Error)
-		} else if r.Error != "" {
-			someFailed = true
-			fmt.Fprintf(serr, "cannot remove %s: %s\n", r.Hash, r.Error)
-		} else {
-			fmt.Fprintf(sout, "removed %s\n", r.Hash)
-		}
-	}
-	if someFailed {
-		return fmt.Errorf("some blocks not removed")
-	}
-	return nil
-}

core/commands/block.go

@@ -8,7 +8,6 @@ import (
 
 	files "github.com/ipfs/go-ipfs-files"
 
-	util "github.com/ipfs/go-ipfs/blocks/blockstoreutil"
 	cmdenv "github.com/ipfs/go-ipfs/core/commands/cmdenv"
 	"github.com/ipfs/go-ipfs/core/commands/cmdutils"
 
@@ -213,6 +212,11 @@ const (
 	blockQuietOptionName = "quiet"
 )
 
+type removedBlock struct {
+	Hash  string `json:",omitempty"`
+	Error string `json:",omitempty"`
+}
+
 var blockRmCmd = &cmds.Command{
 	Helptext: cmds.HelpText{
 		Tagline: "Remove IPFS block(s).",
@@ -246,7 +250,7 @@ It takes a list of base58 encoded multihashes to remove.
 
 			err = api.Block().Rm(req.Context, rp, options.Block.Force(force))
 			if err != nil {
-				if err := res.Emit(&util.RemovedBlock{
+				if err := res.Emit(&removedBlock{
 					Hash:  rp.Cid().String(),
 					Error: err.Error(),
 				}); err != nil {
@@ -256,7 +260,7 @@ It takes a list of base58 encoded multihashes to remove.
 			}
 
 			if !quiet {
-				err := res.Emit(&util.RemovedBlock{
+				err := res.Emit(&removedBlock{
 					Hash: rp.Cid().String(),
 				})
 				if err != nil {
@@ -269,8 +273,29 @@ It takes a list of base58 encoded multihashes to remove.
 	},
 	PostRun: cmds.PostRunMap{
 		cmds.CLI: func(res cmds.Response, re cmds.ResponseEmitter) error {
-			return util.ProcRmOutput(res.Next, os.Stdout, os.Stderr)
+			someFailed := false
+			for {
+				res, err := res.Next()
+				if err == io.EOF {
+					break
+				} else if err != nil {
+					return err
+				}
+				r := res.(*removedBlock)
+				if r.Hash == "" && r.Error != "" {
+					return fmt.Errorf("aborted: %s", r.Error)
+				} else if r.Error != "" {
+					someFailed = true
+					fmt.Fprintf(os.Stderr, "cannot remove %s: %s\n", r.Hash, r.Error)
+				} else {
+					fmt.Fprintf(os.Stdout, "removed %s\n", r.Hash)
+				}
+			}
+			if someFailed {
+				return fmt.Errorf("some blocks not removed")
+			}
+			return nil
 		},
 	},
-	Type: util.RemovedBlock{},
+	Type: removedBlock{},
 }

core/coreapi/block.go

@@ -118,10 +118,8 @@ func (api *BlockAPI) Rm(ctx context.Context, p path.Path, opts ...caopts.BlockRm
 			return errors.New("got unexpected output from util.RmBlocks")
 		}
 
-		// Because errors come as strings we lose information about
-		// the error type.
-		if remBlock.Error != "" {
-			return errors.New(remBlock.Error)
+		if remBlock.Error != nil {
+			return remBlock.Error
 		}
 		return nil
 	case <-ctx.Done():

go.mod

@@ -42,7 +42,7 @@ require (
 	github.com/ipfs/go-ipfs-routing v0.2.1
 	github.com/ipfs/go-ipfs-util v0.0.2
 	github.com/ipfs/go-ipld-cbor v0.0.5
-	github.com/ipfs/go-ipld-format v0.3.0
+	github.com/ipfs/go-ipld-format v0.4.0
 	github.com/ipfs/go-ipld-git v0.1.1
 	github.com/ipfs/go-ipld-legacy v0.1.0
 	github.com/ipfs/go-ipns v0.1.2
@@ -57,7 +57,7 @@ require (
 	github.com/ipfs/go-unixfs v0.3.1
 	github.com/ipfs/go-unixfsnode v1.1.3
 	github.com/ipfs/go-verifcid v0.0.1
-	github.com/ipfs/interface-go-ipfs-core v0.6.0
+	github.com/ipfs/interface-go-ipfs-core v0.6.2
 	github.com/ipfs/tar-utils v0.0.2
 	github.com/ipld/go-car v0.3.2
 	github.com/ipld/go-codec-dagpb v1.3.0

go.sum

@@ -529,8 +529,9 @@ github.com/ipfs/go-ipld-cbor v0.0.5/go.mod h1:BkCduEx3XBCO6t2Sfo5BaHzuok7hbhdMm9
 github.com/ipfs/go-ipld-format v0.0.1/go.mod h1:kyJtbkDALmFHv3QR6et67i35QzO3S0dCDnkOJhcZkms=
 github.com/ipfs/go-ipld-format v0.0.2/go.mod h1:4B6+FM2u9OJ9zCV+kSbgFAZlOrv1Hqbf0INGQgiKf9k=
 github.com/ipfs/go-ipld-format v0.2.0/go.mod h1:3l3C1uKoadTPbeNfrDi+xMInYKlx2Cvg1BuydPSdzQs=
-github.com/ipfs/go-ipld-format v0.3.0 h1:Mwm2oRLzIuUwEPewWAWyMuuBQUsn3awfFEYVb8akMOQ=
 github.com/ipfs/go-ipld-format v0.3.0/go.mod h1:co/SdBE8h99968X0hViiw1MNlh6fvxxnHpvVLnH7jSM=
+github.com/ipfs/go-ipld-format v0.4.0 h1:yqJSaJftjmjc9jEOFYlpkwOLVKv68OD27jFLlSghBlQ=
+github.com/ipfs/go-ipld-format v0.4.0/go.mod h1:co/SdBE8h99968X0hViiw1MNlh6fvxxnHpvVLnH7jSM=
 github.com/ipfs/go-ipld-git v0.1.1 h1:TWGnZjS0htmEmlMFEkA3ogrNCqWjIxwr16x1OsdhG+Y=
 github.com/ipfs/go-ipld-git v0.1.1/go.mod h1:+VyMqF5lMcJh4rwEppV0e6g4nCCHXThLYYDpKUkJubI=
 github.com/ipfs/go-ipld-legacy v0.1.0 h1:wxkkc4k8cnvIGIjPO0waJCe7SHEyFgl+yQdafdjGrpA=
@@ -587,8 +588,8 @@ github.com/ipfs/go-unixfsnode v1.1.3/go.mod h1:ZZxUM5wXBC+G0Co9FjrYTOm+UlhZTjxLf
 github.com/ipfs/go-verifcid v0.0.1 h1:m2HI7zIuR5TFyQ1b79Da5N9dnnCP1vcu2QqawmWlK2E=
 github.com/ipfs/go-verifcid v0.0.1/go.mod h1:5Hrva5KBeIog4A+UpqlaIU+DEstipcJYQQZc0g37pY0=
 github.com/ipfs/interface-go-ipfs-core v0.4.0/go.mod h1:UJBcU6iNennuI05amq3FQ7g0JHUkibHFAfhfUIy927o=
-github.com/ipfs/interface-go-ipfs-core v0.6.0 h1:a43QNc3CNayuMjZM+4D9SeyiSF2nKxBaG8qbCAelMNs=
-github.com/ipfs/interface-go-ipfs-core v0.6.0/go.mod h1:h3NuO3wzv2KuKazt0zDF2/i8AFRqiKHusyh5DUQQdPA=
+github.com/ipfs/interface-go-ipfs-core v0.6.2 h1:nnkq9zhb5O8lPzkZeynEymc83RqkTRqfYH4x5JNUkT4=
+github.com/ipfs/interface-go-ipfs-core v0.6.2/go.mod h1:h3NuO3wzv2KuKazt0zDF2/i8AFRqiKHusyh5DUQQdPA=
 github.com/ipfs/tar-utils v0.0.2 h1:UNgHB4x/PPzbMkmJi+7EqC9LNMPDztOVSnx1HAqSNg4=
 github.com/ipfs/tar-utils v0.0.2/go.mod h1:4qlnRWgTVljIMhSG2SqRYn66NT+3wrv/kZt9V+eqxDM=
 github.com/ipld/go-car v0.3.2 h1:V9wt/80FNfbMRWSD98W5br6fyjUAyVgI2lDOTZX16Lg=

test/sharness/t0050-block.sh

@@ -134,9 +134,9 @@ test_expect_success "multi-block 'ipfs block rm <invalid> <valid> <invalid>'" '
 '
 
 test_expect_success "multi-block 'ipfs block rm <invalid> <valid> <invalid>' output looks good" '
-  echo "cannot remove $RANDOMHASH: $RANDOMHASH not found" >> expect_mixed_rm &&
+  echo "cannot remove $RANDOMHASH: ipld: could not find $RANDOMHASH" >> expect_mixed_rm &&
   echo "removed $TESTHASH" >> expect_mixed_rm &&
-  echo "cannot remove $RANDOMHASH: $RANDOMHASH not found" >> expect_mixed_rm &&
+  echo "cannot remove $RANDOMHASH: ipld: could not find $RANDOMHASH" >> expect_mixed_rm &&
   echo "Error: some blocks not removed" >> expect_mixed_rm
   test_cmp actual_mixed_rm expect_mixed_rm
 '

test/sharness/t0054-dag-car-import-export.sh

@@ -67,7 +67,7 @@ EOE
   # Explainer:
   # naked_root_import_json_expected output is produced by dag import of combined_naked_roots_genesis_and_128.car
   # executed when roots are already present in the repo - thus the BlockCount=0
-  # (if blocks were not present in the repo, blockstore: block not found would be returned)
+  # (if blocks were not present in the repo, ipld: could not find <CID> would be returned)
   cat >naked_root_import_json_expected <<EOE
 {"Root":{"Cid":{"/":"bafy2bzaceaxm23epjsmh75yvzcecsrbavlmkcxnva66bkdebdcnyw3bjrc74u"},"PinErrorMsg":""}}
 {"Root":{"Cid":{"/":"bafy2bzaced4ueelaegfs5fqu4tzsh6ywbbpfk3cxppupmxfdhbpbhzawfw5oy"},"PinErrorMsg":""}}
@@ -179,7 +179,7 @@ test_expect_success "basic offline export of 'getting started' dag works" '
 '
 
 
-echo "Error: block was not found locally (offline): QmYwAPJXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX not found (currently offline, perhaps retry after attaching to the network)" > offline_fetch_error_expected
+echo "Error: block was not found locally (offline): ipld: could not find QmYwAPJXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX (currently offline, perhaps retry after attaching to the network)" > offline_fetch_error_expected
 test_expect_success "basic offline export of nonexistent cid" '
   ! ipfs dag export QmYwAPJXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX 2> offline_fetch_error_actual >/dev/null
 '

test/sharness/t0080-repo.sh

@@ -64,7 +64,7 @@ test_expect_success "'ipfs repo gc --silent' succeeds (no output)" '
   ipfs repo gc --silent >gc_out_empty &&
   test_cmp /dev/null gc_out_empty &&
   test_must_fail ipfs cat "$HASH2" 2>err_expected1 &&
-  grep "Error: $HASH2 not found" err_expected1
+  grep "Error: ipld: could not find $HASH2" err_expected1
 '
 
 test_kill_ipfs_daemon

test/sharness/t0081-repo-pinning.sh

@@ -238,7 +238,7 @@ test_expect_success "some are no longer there" '
 test_launch_ipfs_daemon_without_network
 test_expect_success "recursive pin fails without objects" '
   test_must_fail ipfs pin add -r "$HASH_DIR1" 2>err_expected8 &&
-  grep "not found" err_expected8 ||
+  grep "ipld: could not find" err_expected8 ||
   test_fsh cat err_expected8
 '
 

test/sharness/t0085-pins.sh

@@ -115,7 +115,7 @@ test_pins_error_reporting() {
 
   test_expect_success "'ipfs pin add $PIN_ARGS' on non-existent hash should fail" '
     test_must_fail ipfs pin add $PIN_ARGS $RANDOM_HASH 2> err &&
-    grep -q "not found" err
+    grep -q "ipld: could not find" err
   '
 }
 
@@ -147,7 +147,7 @@ test_pin_dag() {
   test_expect_success "pin file, should fail" '
     test_must_fail ipfs pin add --recursive=true $HASH 2> err &&
     cat err &&
-    grep -q "not found" err
+    grep -q "ipld: could not find" err
   '
 }