fix(list): List datasets even if some refs have bad profileIDs

This works around a problem where users with invalid profileIDs in their repository were unable to list refs at all. The functionality put in place here is only temporary, and will go away once we get rid of CanonicalizeProfile and switch to dscache. For that reason, it doesn't change the interface to lib.List, and instead returns a well-known error for this situation, which cmd uses to output a warning.

api/datasets.go

@@ -213,9 +213,14 @@ func (h *DatasetHandlers) listHandler(w http.ResponseWriter, r *http.Request) {
 
 	res := []dsref.VersionInfo{}
 	if err := h.List(&args, &res); err != nil {
-		log.Infof("error listing datasets: %s", err.Error())
-		util.WriteErrResponse(w, http.StatusInternalServerError, err)
-		return
+		if errors.Is(err, lib.ErrListWarning) {
+			log.Error(err)
+			err = nil
+		} else {
+			log.Infof("error listing datasets: %s", err.Error())
+			util.WriteErrResponse(w, http.StatusInternalServerError, err)
+			return
+		}
 	}
 	if err := util.WritePageResponse(w, res, r, args.Page()); err != nil {
 		log.Infof("error list datasests response: %s", err.Error())

base/dataset.go

@@ -15,6 +15,10 @@ import (
 	reporef "github.com/qri-io/qri/repo/ref"
 )
 
+// ErrUnlistableReferences is an error for when listing references encounters some problem, but
+// these problems are non-fatal
+var ErrUnlistableReferences = errors.New("Warning: Some datasets could not be listed, because of invalid state. These datasets still exist in your repository, but have references that cannot be resolved. This will be fixed in a future version. You can see all datasets by using `qri list --raw`")
+
 // NewLocalDatasetLoader creates a dsfs.Loader that operates on a filestore
 func NewLocalDatasetLoader(r repo.Repo) dsref.Loader {
 	return loader{r}
@@ -183,56 +187,68 @@ func ListDatasets(ctx context.Context, r repo.Repo, term string, limit, offset i
 	}
 	res = res[offset:]
 
-	if limit < len(res) {
-		res = res[:limit]
-	}
+	// Collect references that get resolved and match any given filters
+	matches := make([]reporef.DatasetRef, 0, len(res))
+	hasUnlistableRefs := false
 
-	for i, ref := range res {
-		// May need to change peername.
-		if err := repo.CanonicalizeProfile(r, &res[i]); err != nil {
-			return nil, fmt.Errorf("error canonicalizing dataset peername: %s", err.Error())
+	for _, ref := range res {
+		if err := repo.CanonicalizeProfile(r, &ref); err != nil {
+			// This occurs when two profileIDs map to the same username, which can happen
+			// when a user creates a new profile using an old username. We should ignore
+			// references that can't be resolved this way, since other references in
+			// the repository are still usable
+			hasUnlistableRefs = true
+			continue
+		}
+
+		if term != "" {
+			// If this operation has a term to filter on, skip references that don't match
+			if !strings.Contains(ref.AliasString(), term) {
+				continue
+			}
 		}
 
 		if ref.Path != "" {
 			ds, err := dsfs.LoadDataset(ctx, store, ref.Path)
 			if err != nil {
 				if strings.Contains(err.Error(), "not found") {
-					res[i].Foreign = true
+					ref.Foreign = true
 					err = nil
 					continue
 				}
 				return nil, fmt.Errorf("error loading ref: %s, err: %s", ref.String(), err.Error())
 			}
-			ds.Peername = res[i].Peername
-			ds.Name = res[i].Name
-			res[i].Dataset = ds
+			ds.Peername = ref.Peername
+			ds.Name = ref.Name
+			ref.Dataset = ds
 			if RPC {
-				res[i].Dataset.Structure.Schema = nil
+				ref.Dataset.Structure.Schema = nil
 			}
 
 			if showVersions {
 				dsVersions, err := DatasetLog(ctx, r, reporef.ConvertToDsref(ref), 1000000, 0, false)
 				if err != nil {
 					return nil, err
 				}
-				res[i].Dataset.NumVersions = len(dsVersions)
+				ref.Dataset.NumVersions = len(dsVersions)
 			}
 		}
+
+		matches = append(matches, ref)
 	}
 
-	if term != "" {
-		matched := make([]reporef.DatasetRef, len(res))
-		i := 0
-		for _, ref := range res {
-			if strings.Contains(ref.AliasString(), term) {
-				matched[i] = ref
-				i++
-			}
-		}
-		res = matched[:i]
+	if limit < len(matches) {
+		matches = matches[:limit]
 	}
+	res = matches
 
-	return
+	// If some references could not be listed, return the other, valid references
+	// and return a known error, which callers can handle as desired.
+	if hasUnlistableRefs {
+		return res, ErrUnlistableReferences
+	}
+
+	return res, nil
 }
 
 // RawDatasetRefs converts the dataset refs to a string

cmd/list.go

@@ -3,6 +3,7 @@ package cmd
 import (
 	"bytes"
 	"encoding/json"
+	"errors"
 	"fmt"
 
 	util "github.com/qri-io/apiutil"
@@ -119,7 +120,12 @@ func (o *ListOptions) Run() (err error) {
 		UseDscache:      o.UseDscache,
 	}
 	if err = o.DatasetMethods.List(p, &infos); err != nil {
-		return err
+		if errors.Is(err, lib.ErrListWarning) {
+			printWarning(o.ErrOut, fmt.Sprintf("%s\n", err))
+			err = nil
+		} else {
+			return err
+		}
 	}
 
 	for _, ref := range infos {

lib/datasets.go

@@ -39,6 +39,9 @@ type DatasetMethods struct {
 	inst *Instance
 }
 
+// ErrListWarning is a warning that can occur while listing
+var ErrListWarning = base.ErrUnlistableReferences
+
 // CoreRequestsName implements the Requets interface
 func (DatasetMethods) CoreRequestsName() string { return "datasets" }
 
@@ -81,6 +84,9 @@ func (m *DatasetMethods) List(p *ListParams, res *[]dsref.VersionInfo) error {
 		return err
 	}
 
+	// If the list operation leads to a warning, store it in this var
+	var listWarning error
+
 	var refs []reporef.DatasetRef
 	if p.UseDscache {
 		c := m.inst.dscache
@@ -124,6 +130,10 @@ func (m *DatasetMethods) List(p *ListParams, res *[]dsref.VersionInfo) error {
 		// TODO(dlong): Filtered by p.Published flag
 	} else if ref.Peername == "" || pro.Peername == ref.Peername {
 		refs, err = base.ListDatasets(ctx, m.inst.repo, p.Term, p.Limit, p.Offset, p.RPC, p.Public, p.ShowNumVersions)
+		if errors.Is(err, ErrListWarning) {
+			listWarning = err
+			err = nil
+		}
 	} else {
 		return fmt.Errorf("listing datasets on a peer is not implemented")
 	}
@@ -159,6 +169,10 @@ func (m *DatasetMethods) List(p *ListParams, res *[]dsref.VersionInfo) error {
 	}
 	*res = infos
 
+	if listWarning != nil {
+		err = listWarning
+	}
+
 	return err
 }
 

lib/fsi.go

@@ -395,4 +395,4 @@ func (m *FSIMethods) EnsureRef(p *EnsureParams, out *dsref.VersionInfo) error {
 // PathJoinPosix joins two paths, and makes it explicitly clear we want POSIX slashes
 func PathJoinPosix(left, right string) string {
 	return path.Join(left, right)
-}
+}

lib/lib_test.go

@@ -249,10 +249,10 @@ func addNowTransformDataset(t *testing.T, node *p2p.QriNode) dsref.Ref {
 }
 
 func TestInstanceEventSubscription(t *testing.T) {
-	timeoutMs := time.Millisecond*250
+	timeoutMs := time.Millisecond * 250
 	if runtime.GOOS == "windows" {
 		// TODO(dustmop): Why is windows slow? Perhaps its due to the IPFS lock.
-		timeoutMs = time.Millisecond*2500
+		timeoutMs = time.Millisecond * 2500
 	}
 	// TODO (b5) - can't use testrunner for this test because event busses aren't
 	// wired up correctly in the test runner constructor. The proper fix is to have