Skip to content

Commit

Permalink
fix: make document::Create more resilient if subsequent patch call re…
Browse files Browse the repository at this point in the history
…ceives 404

We call PATCH immediately after POST when creating a document in order to make it public.
However, it can be the case that the subsequent call to the PATCH endpoint happens to early. In this situation the API returns 404 and the call to Create fails. To make this more resilient, a retry mechanism was added.

Signed-off-by: Bernd Warmuth <[email protected]>
  • Loading branch information
Bernd Warmuth authored and warber committed Jul 24, 2024
1 parent ccc0481 commit e11e82a
Show file tree
Hide file tree
Showing 2 changed files with 62 additions and 4 deletions.
28 changes: 24 additions & 4 deletions clients/documents/documents.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (
"net/http"
"net/url"
"strings"
"time"

"github.com/dynatrace/dynatrace-configuration-as-code-core/api"
"github.com/dynatrace/dynatrace-configuration-as-code-core/api/clients/documents"
Expand Down Expand Up @@ -215,14 +216,15 @@ func (c Client) Create(ctx context.Context, name string, isPrivate bool, externa
return api.Response{}, err
}

r, err := c.patch(ctx, md.ID, md.Version, d)
r, err := c.patchWithRetry(ctx, md.ID, md.Version, d)
if err != nil {
if _, err1 := c.delete(ctx, md.ID, md.Version); err1 != nil {
return api.Response{}, errors.Join(err, err1)
if !isNotFoundError(err) {
if _, err1 := c.delete(ctx, md.ID, md.Version); err1 != nil {
return api.Response{}, errors.Join(err, err1)
}
}
return api.Response{}, err
}

return r, nil
}

Expand Down Expand Up @@ -275,6 +277,24 @@ func (c Client) create(ctx context.Context, d documents.Document) (api.Response,
return processHttpResponse(c.client.Create(ctx, d))
}

func (c Client) patchWithRetry(ctx context.Context, id string, version int, d documents.Document) (resp api.Response, err error) {
const maxRetries = 5
const retryDelay = 200 * time.Millisecond
for r := 0; r < maxRetries; r++ {
if resp, err = c.patch(ctx, id, version, d); isNotFoundError(err) {
time.Sleep(retryDelay)
continue
}
break
}
return
}

func isNotFoundError(err error) bool {
var apiErr api.APIError
return errors.As(err, &apiErr) && apiErr.StatusCode == http.StatusNotFound
}

func (c Client) patch(ctx context.Context, id string, version int, d documents.Document) (api.Response, error) {
resp, err := processHttpResponse(c.client.Patch(ctx, id, version, d))
if err != nil {
Expand Down
38 changes: 38 additions & 0 deletions clients/documents/documents_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,44 @@ func TestDocumentClient_Create(t *testing.T) {
assert.ErrorContains(t, err, "some internal error")
})

t.Run("patch call returns 404 - retry succeeds", func(t *testing.T) {
ctx := testutils.ContextWithLogger(t)
mockClient := documents.NewMockclient(gomock.NewController(t))
mockClient.EXPECT().Create(ctx, givenDoc).
Return(&http.Response{Status: http.StatusText(http.StatusCreated), StatusCode: http.StatusCreated, Body: io.NopCloser(strings.NewReader(respCreate)), Request: &http.Request{Method: http.MethodGet, URL: &url.URL{}}}, nil)
mockClient.EXPECT().Patch(ctx, "f6e26fdd-1451-4655-b6ab-1240a00c1fba", 1, givenDoc).Times(4).
Return(&http.Response{Status: http.StatusText(http.StatusNotFound), StatusCode: http.StatusNotFound, Body: io.NopCloser(strings.NewReader("some internal error")), Request: &http.Request{Method: http.MethodPatch, URL: &url.URL{}}}, nil)
mockClient.EXPECT().Patch(ctx, "f6e26fdd-1451-4655-b6ab-1240a00c1fba", 1, givenDoc).
Return(&http.Response{Status: http.StatusText(http.StatusOK), StatusCode: http.StatusOK, Body: io.NopCloser(strings.NewReader(respPatch)), Request: &http.Request{Method: http.MethodPatch, URL: &url.URL{}}}, nil)

docClient := documents.NewTestClient(mockClient)

res, err := docClient.Create(ctx, "name", false, "extID", []byte("this is the content"), documents.Notebook)

require.NoError(t, err)
assert.JSONEq(t, expected, string(res.Data))
})

t.Run("patch call returns 404 - retry fails", func(t *testing.T) {
ctx := testutils.ContextWithLogger(t)
mockClient := documents.NewMockclient(gomock.NewController(t))
mockClient.EXPECT().Create(ctx, givenDoc).
Return(&http.Response{Status: http.StatusText(http.StatusCreated), StatusCode: http.StatusCreated, Body: io.NopCloser(strings.NewReader(respCreate)), Request: &http.Request{Method: http.MethodGet, URL: &url.URL{}}}, nil)
mockClient.EXPECT().Patch(ctx, "f6e26fdd-1451-4655-b6ab-1240a00c1fba", 1, givenDoc).Times(5).
Return(&http.Response{Status: http.StatusText(http.StatusNotFound), StatusCode: http.StatusNotFound, Body: io.NopCloser(strings.NewReader("some internal error")), Request: &http.Request{Method: http.MethodPatch, URL: &url.URL{}}}, nil)

docClient := documents.NewTestClient(mockClient)

res, err := docClient.Create(ctx, "name", false, "extID", []byte("this is the content"), documents.Notebook)

require.Empty(t, res)
require.Error(t, err)

// var apiErr api.APIError
assert.ErrorAs(t, err, &api.APIError{})
assert.ErrorContains(t, err, "API request HTTP PATCH failed with status code 404")
})

t.Run("patch call returns non successful response; rollback fails", func(t *testing.T) {
ctx := testutils.ContextWithLogger(t)
mockClient := documents.NewMockclient(gomock.NewController(t))
Expand Down

0 comments on commit e11e82a

Please sign in to comment.