Don't omit trailing error response for fake pages (Azure#21849)

Fetching the next page, or an error, is predicated on the page
response's next link field being populated.
Always set the next link field even if the next "page" is an error
response.

Author: jhendrixMSFT

sdk/azcore/CHANGELOG.md

@@ -16,6 +16,7 @@
 * Block key and SAS authentication for non TLS protected endpoints.
 * Passing a `nil` credential value will no longer cause a panic. Instead, the authentication is skipped.
 * Calling `Error` on a zero-value `azcore.ResponseError` will no longer panic.
+* Fixed an issue in `fake.PagerResponder[T]` that would cause a trailing error to be omitted when iterating over pages.
 
 ### Other Changes
 

sdk/azcore/fake/fake.go

@@ -74,7 +74,9 @@ func (e *ErrorResponder) SetResponseError(httpStatus int, errorCode string) {
 /////////////////////////////////////////////////////////////////////////////////////////////////////////////
 
 // PagerResponder represents a sequence of paged responses.
-// Responses are replayed in the order in which they were added.
+// Responses are consumed in the order in which they were added.
+// If no pages or errors have been added, calls to Pager[T].NextPage
+// will return an error.
 type PagerResponder[T any] exported.PagerResponder[T]
 
 // AddPage adds a page to the sequence of respones.
@@ -102,8 +104,10 @@ type AddPageOptions = exported.AddPageOptions
 /////////////////////////////////////////////////////////////////////////////////////////////////////////////
 
 // PollerResponder represents a sequence of responses for a long-running operation.
-// Any non-terminal responses are replayed in the order in which they were added.
+// Any non-terminal responses are consumed in the order in which they were added.
 // The terminal response, success or error, is always the final response.
+// If no responses or errors have been added, the following method calls on Poller[T]
+// will return an error: PollUntilDone, Poll, Result.
 type PollerResponder[T any] exported.PollerResponder[T]
 
 // AddNonTerminalResponse adds a non-terminal response to the sequence of responses.

sdk/azcore/fake/fake_test.go

@@ -161,7 +161,7 @@ func TestPagerResponder(t *testing.T) {
 			require.NotNil(t, resp)
 			page, err := unmarshal[widgets](resp)
 			require.NoError(t, err)
-			require.Nil(t, page.NextPage)
+			require.NotNil(t, page.NextPage)
 			require.Equal(t, []widget{{Name: "baz"}}, page.Widgets)
 		case 4:
 			require.Error(t, err)

sdk/azcore/fake/internal/exported/fake.go

@@ -133,7 +133,7 @@ type AddPageOptions struct {
 // This function is called by the fake server internals.
 func (p *PagerResponder[T]) Next(req *http.Request) (*http.Response, error) {
 	if len(p.pages) == 0 {
-		return nil, shared.NonRetriableError(errors.New("paged response has no pages"))
+		return nil, shared.NonRetriableError(errors.New("fake paged response is empty"))
 	}
 
 	page := p.pages[0]
@@ -175,48 +175,42 @@ func (p *PagerResponder[T]) More() bool {
 	return len(p.pages) > 0
 }
 
-type pageindex[T any] struct {
-	i    int
-	page pageResp[T]
-}
-
 // nextLinkURLSuffix is the URL path suffix for a faked next page followed by one or more digits.
 const nextLinkURLSuffix = "/fake_page_"
 
 // InjectNextLinks is used to populate the nextLink field.
 // The inject callback is executed for every T in the sequence except for the last one.
 // This function is called by the fake server internals.
 func (p *PagerResponder[T]) InjectNextLinks(req *http.Request, inject func(page *T, createLink func() string)) {
-	// first find all the actual pages in the list
-	pages := make([]pageindex[T], 0, len(p.pages))
+	// populate the next links, including pageResp[T] where the next
+	// "page" is an error response. this allows an error response to
+	// be returned when there are no subsequent pages.
+	pageNum := 1
 	for i := range p.pages {
-		if pageT, ok := p.pages[i].(pageResp[T]); ok {
-			pages = append(pages, pageindex[T]{
-				i:    i,
-				page: pageT,
-			})
-		}
-	}
-
-	// now populate the next links
-	for i := range pages {
-		if i+1 == len(pages) {
+		if i+1 == len(p.pages) {
 			// no nextLink for last page
 			break
 		}
 
+		pageT, ok := p.pages[i].(pageResp[T])
+		if !ok {
+			// error entry, no next link
+			continue
+		}
+
 		qp := ""
 		if req.URL.RawQuery != "" {
 			qp = "?" + req.URL.RawQuery
 		}
 
-		inject(&pages[i].page.entry, func() string {
+		inject(&pageT.entry, func() string {
 			// NOTE: any changes to this path format MUST be reflected in SanitizePagerPath()
-			return fmt.Sprintf("%s://%s%s%s%d%s", req.URL.Scheme, req.URL.Host, req.URL.Path, nextLinkURLSuffix, i+1, qp)
+			return fmt.Sprintf("%s://%s%s%s%d%s", req.URL.Scheme, req.URL.Host, req.URL.Path, nextLinkURLSuffix, pageNum, qp)
 		})
+		pageNum++
 
 		// update the original slice with the modified page
-		p.pages[pages[i].i] = pages[i].page
+		p.pages[i] = pageT
 	}
 }
 
@@ -326,7 +320,7 @@ func (p *PollerResponder[T]) Next(req *http.Request) (*http.Response, error) {
 		httpResp.Header.Set(shared.HeaderFakePollerStatus, "Succeeded")
 		return httpResp, nil
 	} else {
-		return nil, shared.NonRetriableError(fmt.Errorf("%T has no terminal response", p))
+		return nil, shared.NonRetriableError(errors.New("fake poller response is emtpy"))
 	}
 }
 

sdk/azcore/fake/internal/exported/fake_test.go

@@ -134,7 +134,7 @@ func TestPagerResponder(t *testing.T) {
 			require.NotNil(t, resp)
 			page, err := unmarshal[widgets](resp)
 			require.NoError(t, err)
-			require.Nil(t, page.NextPage)
+			require.NotNil(t, page.NextPage)
 			require.Equal(t, []widget{{Name: "baz"}}, page.Widgets)
 		case 4:
 			require.Error(t, err)
@@ -149,6 +149,104 @@ func TestPagerResponder(t *testing.T) {
 		iterations++
 	}
 	require.Equal(t, 5, iterations)
+
+	// single page with subsequent error
+	pagerResp = PagerResponder[widgets]{}
+
+	pagerResp.AddPage(http.StatusOK, widgets{
+		Widgets: []widget{
+			{Name: "foo"},
+			{Name: "bar"},
+		},
+	}, nil)
+	pagerResp.AddError(errors.New("two"))
+
+	pagerResp.InjectNextLinks(req, func(p *widgets, create func() string) {
+		p.NextPage = to.Ptr(create())
+	})
+
+	iterations = 0
+	for pagerResp.More() {
+		resp, err := pagerResp.Next(req)
+		switch iterations {
+		case 0:
+			require.NoError(t, err)
+			require.NotNil(t, resp)
+			page, err := unmarshal[widgets](resp)
+			require.NoError(t, err)
+			require.NotNil(t, page.NextPage)
+			require.Equal(t, []widget{{Name: "foo"}, {Name: "bar"}}, page.Widgets)
+		case 1:
+			require.Error(t, err)
+			require.Nil(t, resp)
+		}
+		iterations++
+	}
+	require.EqualValues(t, 2, iterations)
+
+	// single page with subsequent response error
+	pagerResp = PagerResponder[widgets]{}
+
+	pagerResp.AddPage(http.StatusOK, widgets{
+		Widgets: []widget{
+			{Name: "foo"},
+			{Name: "bar"},
+		},
+	}, nil)
+	pagerResp.AddResponseError(http.StatusBadRequest, "BadRequest")
+
+	pagerResp.InjectNextLinks(req, func(p *widgets, create func() string) {
+		p.NextPage = to.Ptr(create())
+	})
+
+	iterations = 0
+	for pagerResp.More() {
+		resp, err := pagerResp.Next(req)
+		switch iterations {
+		case 0:
+			require.NoError(t, err)
+			require.NotNil(t, resp)
+			page, err := unmarshal[widgets](resp)
+			require.NoError(t, err)
+			require.NotNil(t, page.NextPage)
+			require.Equal(t, []widget{{Name: "foo"}, {Name: "bar"}}, page.Widgets)
+		case 1:
+			require.Error(t, err)
+			require.Nil(t, resp)
+		}
+		iterations++
+	}
+	require.EqualValues(t, 2, iterations)
+
+	// single page
+	pagerResp = PagerResponder[widgets]{}
+
+	pagerResp.AddPage(http.StatusOK, widgets{
+		Widgets: []widget{
+			{Name: "foo"},
+			{Name: "bar"},
+		},
+	}, nil)
+
+	pagerResp.InjectNextLinks(req, func(p *widgets, create func() string) {
+		p.NextPage = to.Ptr(create())
+	})
+
+	iterations = 0
+	for pagerResp.More() {
+		resp, err := pagerResp.Next(req)
+		switch iterations {
+		case 0:
+			require.NoError(t, err)
+			require.NotNil(t, resp)
+			page, err := unmarshal[widgets](resp)
+			require.NoError(t, err)
+			require.Nil(t, page.NextPage)
+			require.Equal(t, []widget{{Name: "foo"}, {Name: "bar"}}, page.Widgets)
+		}
+		iterations++
+	}
+	require.EqualValues(t, 1, iterations)
 }
 
 func TestPollerResponder(t *testing.T) {