Check for Sync-Token before updating the cache (Azure#21600)

Some responses, e.g. http.StatusBadRequest, won't return a Sync-Token.
azure-sdk · Sep 20, 2023 · adeb6d3 · adeb6d3
1 parent 9464951
commit adeb6d3
Show file tree

Hide file tree

Showing 3 changed files with 11 additions and 6 deletions.
diff --git a/sdk/data/azappconfig/CHANGELOG.md b/sdk/data/azappconfig/CHANGELOG.md
@@ -7,6 +7,7 @@
 ### Breaking Changes
 
 ### Bugs Fixed
+* Check for a `Sync-Token` value before updating the cache.
 
 ### Other Changes
 

diff --git a/sdk/data/azappconfig/internal/synctoken/policy.go b/sdk/data/azappconfig/internal/synctoken/policy.go
@@ -39,9 +39,12 @@ func (p *Policy) Do(req *policy.Request) (*http.Response, error) {
 		return nil, err
 	}
 
-	// update the cache from the response
-	if err := p.cache.Set(exported.SyncToken(resp.Header.Get(syncTokenHeader))); err != nil {
-		return nil, &nonRetriableError{err}
+	// update the cache from the response if available.
+	// e.g. a 404 will include a Sync-Token but a 400 will not.
+	if st := resp.Header.Get(syncTokenHeader); st != "" {
+		if err := p.cache.Set(exported.SyncToken(st)); err != nil {
+			return nil, &nonRetriableError{err}
+		}
 	}
 
 	return resp, err

diff --git a/sdk/data/azappconfig/internal/synctoken/policy_test.go b/sdk/data/azappconfig/internal/synctoken/policy_test.go
@@ -19,7 +19,6 @@ import (
 
 func TestPolicy(t *testing.T) {
 	srv, close := mock.NewServer()
-	srv.AppendResponse()
 	defer close()
 
 	cache := NewCache()
@@ -36,9 +35,11 @@ func TestPolicy(t *testing.T) {
 
 	req, err := runtime.NewRequest(context.Background(), http.MethodGet, srv.URL())
 	require.NoError(t, err)
+
+	srv.AppendResponse()
 	resp, err := pl.Do(req)
-	require.Error(t, err) // missing Sync-Token response header
-	require.Nil(t, resp)
+	require.NoError(t, err)
+	require.NotNil(t, resp)
 
 	srv.AppendResponse(mock.WithHeader(syncTokenHeader, "id=val"))
 	resp, err = pl.Do(req)