15864: Fix error response when requesting nonexistent UUIDs.
authorTom Clegg <tclegg@veritasgenetics.com>
Fri, 29 Nov 2019 07:37:41 +0000 (02:37 -0500)
committerTom Clegg <tclegg@veritasgenetics.com>
Fri, 29 Nov 2019 07:37:41 +0000 (02:37 -0500)
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg@veritasgenetics.com>

lib/controller/federation/list.go
lib/controller/federation/list_test.go

index 26b6b254e8e9fbdbb59638ca441412ade5575abb..8650491ddc30e03766e72777626ee86216056225 100644 (file)
@@ -237,7 +237,7 @@ func (conn *Conn) splitListRequest(ctx context.Context, opts arvados.ListOptions
 
                                done, err := fn(ctx, clusterID, backend, remoteOpts)
                                if err != nil {
-                                       errs <- err
+                                       errs <- httpErrorf(http.StatusBadGateway, err.Error())
                                        return
                                }
                                progress := false
@@ -247,8 +247,13 @@ func (conn *Conn) splitListRequest(ctx context.Context, opts arvados.ListOptions
                                                delete(todo, uuid)
                                        }
                                }
-                               if !progress {
-                                       errs <- httpErrorf(http.StatusBadGateway, "cannot make progress in federated list query: cluster %q returned none of the requested UUIDs", clusterID)
+                               if len(done) == 0 {
+                                       // Zero items == no more
+                                       // results exist, no need to
+                                       // get another page.
+                                       break
+                               } else if !progress {
+                                       errs <- httpErrorf(http.StatusBadGateway, "cannot make progress in federated list query: cluster %q returned %d items but none had the requested UUIDs", clusterID, len(done))
                                        return
                                }
                        }
index a9c4f588f12a38b017d1c89b2995263a8c12d3d6..30749aac316b5f7a409bf8afa48439e3c6b68bac 100644 (file)
@@ -338,7 +338,7 @@ func (s *CollectionListSuite) TestCollectionListRemoteUnknown(c *check.C) {
 }
 
 func (s *CollectionListSuite) TestCollectionListRemoteError(c *check.C) {
-       s.addDirectRemote(c, "bbbbb", &arvadostest.APIStub{})
+       s.addDirectRemote(c, "bbbbb", &arvadostest.APIStub{Error: fmt.Errorf("stub backend error")})
        s.test(c, listTrial{
                count: "none",
                limit: -1,
@@ -359,8 +359,8 @@ func (s *CollectionListSuite) test(c *check.C, trial listTrial) {
        })
        if trial.expectStatus != 0 {
                c.Assert(err, check.NotNil)
-               err, _ := err.(interface{ HTTPStatus() int })
-               c.Assert(err, check.NotNil) // err must implement HTTPStatus()
+               err, ok := err.(interface{ HTTPStatus() int })
+               c.Assert(ok, check.Equals, true) // err must implement interface{ HTTPStatus() int }
                c.Check(err.HTTPStatus(), check.Equals, trial.expectStatus)
                c.Logf("returned error is %#v", err)
                c.Logf("returned error string is %q", err)