address review comments

Author: rosstimothy

lib/backend/backend.go

@@ -117,14 +117,15 @@ type Backend interface {
 // IterateParams are parameters that are provided to
 // [BackendWithItems.Items] to alter the iteration behavior.
 type IterateParams struct {
-	// StartKey is the key at which to begin iteration. This key
+	// StartKey is the minimum key in the range yielded by the iteration. This key
 	// will be included in the results if it exists.
 	StartKey Key
-	// EndKey is the key at which to end iteration. This key
+	// EndKey is the maximum key in the range yielded by the iteration. This key
 	// will be included in the results if it exists.
 	EndKey Key
-	// Descending indicates which direction the items should be produced,
-	// by default from [StartKey] to [EndKey].
+	// Descending makes the iteration yield items from the biggest to the smallest
+	// key (i.e. from EndKey to StartKey). If unset, the iteration will proceed in the
+	// usual ascending order (i.e. from StartKey to EndKey).
 	Descending bool
 	// Limit is an optional maximum number of items to retrieve during iteration.
 	Limit int

lib/backend/etcdbk/etcd.go

@@ -689,10 +689,9 @@ func (b *EtcdBackend) Items(ctx context.Context, params backend.IterateParams) i
 			batchReadLatencies.Observe(time.Since(start).Seconds())
 			batchReadRequests.Inc()
 			if err := convertErr(err); err != nil {
-				if yield(backend.Item{}, trace.Wrap(err)) {
-					b.logger.ErrorContext(ctx, "items iteration allowed to continue after encountering error (this is a bug)", "error", err)
+				if !yield(backend.Item{}, trace.Wrap(err)) {
+					return
 				}
-				return
 			}
 
 			if len(re.Kvs) == 0 {
@@ -702,10 +701,9 @@ func (b *EtcdBackend) Items(ctx context.Context, params backend.IterateParams) i
 			for _, kv := range re.Kvs {
 				value, err := unmarshal(kv.Value)
 				if err != nil {
-					if yield(backend.Item{}, trace.Wrap(err)) {
-						b.logger.ErrorContext(ctx, "items iteration allowed to continue after encountering error (this is a bug)", "error", err)
+					if !yield(backend.Item{}, trace.Wrap(err)) {
+						return
 					}
-					return
 				}
 
 				if !yield(backend.Item{
@@ -725,7 +723,7 @@ func (b *EtcdBackend) Items(ctx context.Context, params backend.IterateParams) i
 			if params.Descending {
 				endKey = string(re.Kvs[len(re.Kvs)-1].Key)
 			} else {
-				inclusiveStartKey = backend.RangeEnd(backend.KeyFromString(string(re.Kvs[len(re.Kvs)-1].Key))).String()
+				inclusiveStartKey = string(re.Kvs[len(re.Kvs)-1].Key) + "\x00"
 			}
 		}
 	}

lib/backend/test/suite.go

@@ -567,7 +567,6 @@ func testItems(t *testing.T, newBackend Constructor) {
 		t.Run("descending", func(t *testing.T) {
 			i := count - 1
 			for item := range it.Items(ctx, backend.IterateParams{StartKey: prefix("page"), EndKey: backend.RangeEnd(prefix("page")), Descending: true}) {
-				t.Logf("expected=%s|actual=%s", expected[i], string(item.Value))
 				assert.Equal(t, expected[i], string(item.Value))
 				i--
 			}