From c9254bff1b801a77534d99b42c2f605925d9cd0a Mon Sep 17 00:00:00 2001 From: Debaditya Date: Mon, 9 Dec 2024 12:01:36 +0530 Subject: [PATCH 1/5] Added resource error details for polling state --- client/build.go | 9 ++++++--- client/job.go | 10 +++++++--- client/package.go | 6 +++--- client/polling.go | 6 +++--- client/polling_test.go | 20 ++++++++++++-------- operation/push.go | 6 +++--- 6 files changed, 34 insertions(+), 23 deletions(-) diff --git a/client/build.go b/client/build.go index a8743f60..2d3b3aa0 100644 --- a/client/build.go +++ b/client/build.go @@ -138,12 +138,15 @@ func (c *BuildClient) ListForAppAll(ctx context.Context, appGUID string, opts *B // PollStaged waits until the build is staged, fails, or times out func (c *BuildClient) PollStaged(ctx context.Context, guid string, opts *PollingOptions) error { - return PollForStateOrTimeout(func() (string, error) { + return PollForStateOrTimeout(func() (string, string, error) { build, err := c.Get(ctx, guid) if build != nil { - return string(build.State), err + if build.Error != nil { + return string(build.State), *build.Error, err + } + return string(build.State), "", err } - return "", err + return "", "", err }, string(resource.BuildStateStaged), opts) } diff --git a/client/job.go b/client/job.go index bf7bfb79..3cdb2955 100644 --- a/client/job.go +++ b/client/job.go @@ -21,12 +21,16 @@ func (c *JobClient) Get(ctx context.Context, guid string) (*resource.Job, error) // PollComplete waits until the job completes, fails, or times out func (c *JobClient) PollComplete(ctx context.Context, jobGUID string, opts *PollingOptions) error { - err := PollForStateOrTimeout(func() (string, error) { + err := PollForStateOrTimeout(func() (string, string, error) { job, err := c.Get(ctx, jobGUID) if job != nil { - return string(job.State), err + var cfErrors string + for _, error := range job.Errors { + cfErrors = cfErrors + "\n" + error.Error() + } + return string(job.State), cfErrors, err } - return "", err + return "", "", err }, string(resource.JobStateComplete), opts) // attempt to return the underlying saved job error diff --git a/client/package.go b/client/package.go index 5743d1ad..14b5561c 100644 --- a/client/package.go +++ b/client/package.go @@ -143,12 +143,12 @@ func (c *PackageClient) ListForAppAll(ctx context.Context, appGUID string, opts // PollReady waits until the package is ready, fails, or times out func (c *PackageClient) PollReady(ctx context.Context, guid string, opts *PollingOptions) error { - return PollForStateOrTimeout(func() (string, error) { + return PollForStateOrTimeout(func() (string, string, error) { pkg, err := c.Get(ctx, guid) if pkg != nil { - return string(pkg.State), err + return string(pkg.State), "", err } - return "", err + return "", "", err }, string(resource.PackageStateReady), opts) } diff --git a/client/polling.go b/client/polling.go index 580dea2d..b55d585e 100644 --- a/client/polling.go +++ b/client/polling.go @@ -22,7 +22,7 @@ func NewPollingOptions() *PollingOptions { } } -type getStateFunc func() (string, error) +type getStateFunc func() (string, string, error) func PollForStateOrTimeout(getState getStateFunc, successState string, opts *PollingOptions) error { if opts == nil { @@ -38,7 +38,7 @@ func PollForStateOrTimeout(getState getStateFunc, successState string, opts *Pol case <-timeout: return AsyncProcessTimeoutError case <-ticker.C: - state, err := getState() + state, cferror, err := getState() if err != nil { return err } @@ -46,7 +46,7 @@ func PollForStateOrTimeout(getState getStateFunc, successState string, opts *Pol case successState: return nil case opts.FailedState: - return AsyncProcessFailedError + return errors.Join(AsyncProcessFailedError, errors.New(cferror)) } } } diff --git a/client/polling_test.go b/client/polling_test.go index 1156bd3d..be1e416d 100644 --- a/client/polling_test.go +++ b/client/polling_test.go @@ -1,11 +1,15 @@ package client import ( - "github.com/stretchr/testify/require" + "errors" "testing" "time" + + "github.com/stretchr/testify/require" ) +var CustomStagingErr = "StagingError - Staging error: Start command not specified" + func TestNewPollingOptions(t *testing.T) { opts := NewPollingOptions() require.Equal(t, "FAILED", opts.FailedState) @@ -18,18 +22,18 @@ func TestPollForStateOrTimeout(t *testing.T) { noWaitOpts.Timeout = time.Second noWaitOpts.CheckInterval = time.Millisecond - failedFn := func() (string, error) { - return "FAILED", nil + failedFn := func() (string, string, error) { + return "FAILED", CustomStagingErr, nil } - successFn := func() (string, error) { - return "SUCCESS", nil + successFn := func() (string, string, error) { + return "SUCCESS", "", nil } - timeoutFn := func() (string, error) { - return "PROCESSING", nil + timeoutFn := func() (string, string, error) { + return "PROCESSING", "", nil } err := PollForStateOrTimeout(failedFn, "NOPE", noWaitOpts) - require.Equal(t, AsyncProcessFailedError, err) + require.Equal(t, errors.Join(AsyncProcessFailedError, errors.New(CustomStagingErr)), err) err = PollForStateOrTimeout(successFn, "SUCCESS", noWaitOpts) require.NoError(t, err) diff --git a/operation/push.go b/operation/push.go index c5035dad..f81c1191 100644 --- a/operation/push.go +++ b/operation/push.go @@ -187,12 +187,12 @@ func (p *AppPushOperation) waitForDeployment(ctx context.Context, deploymentGUID pollOptions := client.NewPollingOptions() pollOptions.Timeout = time.Duration(instances) * time.Minute - depPollErr := client.PollForStateOrTimeout(func() (string, error) { + depPollErr := client.PollForStateOrTimeout(func() (string, string, error) { deployment, err := p.client.Deployments.Get(ctx, deploymentGUID) if err != nil { - return "", err + return "", "", err } - return deployment.Status.Value, nil + return deployment.Status.Value, deployment.Status.Reason, nil }, "FINALIZED", pollOptions) return depPollErr } From d3ab46978286a404b24e890177fd747b86bf2d0f Mon Sep 17 00:00:00 2001 From: Shawn Neal Date: Fri, 14 Feb 2025 13:15:06 -0800 Subject: [PATCH 2/5] Add job polling tests --- client/job_test.go | 42 ++++++++++++++++++++++++++++++- testutil/object_generator.go | 7 ++++++ testutil/template/job_failed.json | 29 +++++++++++++++++++++ 3 files changed, 77 insertions(+), 1 deletion(-) create mode 100644 testutil/template/job_failed.json diff --git a/client/job_test.go b/client/job_test.go index 8e4b411d..76141ab5 100644 --- a/client/job_test.go +++ b/client/job_test.go @@ -2,8 +2,10 @@ package client import ( "context" + "github.com/stretchr/testify/require" "net/http" "testing" + "time" "github.com/cloudfoundry/go-cfclient/v3/testutil" ) @@ -11,6 +13,13 @@ import ( func TestJobs(t *testing.T) { g := testutil.NewObjectJSONGenerator(1) job := g.Job("COMPLETE").JSON + jobProcessing := g.Job("PROCESSING").JSON + jobFailed := g.JobFailed().JSON + pollingOpts := &PollingOptions{ + FailedState: "FAILED", + Timeout: time.Second, + CheckInterval: time.Nanosecond, + } tests := []RouteTest{ { @@ -19,12 +28,43 @@ func TestJobs(t *testing.T) { Method: "GET", Endpoint: "/v3/jobs/c33a5caf-77e0-4d6e-b587-5555d339bc9a", Output: g.Single(job), - Status: http.StatusOK}, + Status: http.StatusOK, + }, Expected: job, Action: func(c *Client, t *testing.T) (any, error) { return c.Jobs.Get(context.Background(), "c33a5caf-77e0-4d6e-b587-5555d339bc9a") }, }, + { + Description: "Poll job that succeeds", + Route: testutil.MockRoute{ + Method: "GET", + Endpoint: "/v3/jobs/c33a5caf-77e0-4d6e-b587-5555d339bc9a", + Output: []string{jobProcessing, job}, + Statuses: []int{http.StatusOK, http.StatusOK}, + }, + Action: func(c *Client, t *testing.T) (any, error) { + err := c.Jobs.PollComplete(context.Background(), "c33a5caf-77e0-4d6e-b587-5555d339bc9a", pollingOpts) + return nil, err + }, + }, + { + Description: "Poll job that fails", + Route: testutil.MockRoute{ + Method: "GET", + Endpoint: "/v3/jobs/40e49716-44a3-4ae9-9926-d1a804acf70c", + Output: []string{jobProcessing, jobFailed}, + Statuses: []int{http.StatusOK, http.StatusOK}, + }, + Action: func(c *Client, t *testing.T) (any, error) { + err := c.Jobs.PollComplete(context.Background(), "40e49716-44a3-4ae9-9926-d1a804acf70c", pollingOpts) + require.Error(t, err) + require.ErrorContains(t, err, "received state FAILED while waiting for async process") + require.ErrorContains(t, err, "cfclient error (CF-UnprocessableEntity|10008): something went wrong") + require.ErrorContains(t, err, "cfclient error (UnknownError|10001): unexpected error occurred") + return nil, nil + }, + }, } ExecuteTests(tests, t) } diff --git a/testutil/object_generator.go b/testutil/object_generator.go index 281844fb..6726d52d 100644 --- a/testutil/object_generator.go +++ b/testutil/object_generator.go @@ -228,6 +228,13 @@ func (o ObjectJSONGenerator) Job(state string) *JSONResource { return o.renderTemplate(r, "job.json") } +func (o ObjectJSONGenerator) JobFailed() *JSONResource { + r := &JSONResource{ + GUID: RandomGUID(), + } + return o.renderTemplate(r, "job_failed.json") +} + func (o ObjectJSONGenerator) Manifest() *JSONResource { r := &JSONResource{} return o.renderTemplate(r, "manifest.yml") diff --git a/testutil/template/job_failed.json b/testutil/template/job_failed.json new file mode 100644 index 00000000..59e5f24d --- /dev/null +++ b/testutil/template/job_failed.json @@ -0,0 +1,29 @@ +{ + "guid": "{{.GUID}}", + "created_at": "2016-10-19T20:25:04Z", + "updated_at": "2016-11-08T16:41:26Z", + "operation": "app.create", + "state": "FAILED", + "links": { + "self": { + "href": "https://api.example.org/v3/jobs/{{.GUID}}" + } + }, + "errors": [ + { + "code": 10008, + "title": "CF-UnprocessableEntity", + "detail": "something went wrong" + }, + { + "code": 10001, + "title": "UnknownError", + "detail": "unexpected error occurred" + } + ], + "warnings": [ + { + "detail": "some warning" + } + ] +} \ No newline at end of file From bee15db3d6b00a3dedd0ac210fd6191881c1e166 Mon Sep 17 00:00:00 2001 From: Shawn Neal Date: Fri, 14 Feb 2025 14:43:34 -0800 Subject: [PATCH 3/5] Add job warnings to returned error messages --- client/job.go | 7 +++++-- client/job_test.go | 1 + 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/client/job.go b/client/job.go index 3cdb2955..5008acd1 100644 --- a/client/job.go +++ b/client/job.go @@ -25,8 +25,11 @@ func (c *JobClient) PollComplete(ctx context.Context, jobGUID string, opts *Poll job, err := c.Get(ctx, jobGUID) if job != nil { var cfErrors string - for _, error := range job.Errors { - cfErrors = cfErrors + "\n" + error.Error() + for _, e := range job.Errors { + cfErrors += "\n" + e.Error() + } + for _, e := range job.Warnings { + cfErrors += "\n" + e.Detail } return string(job.State), cfErrors, err } diff --git a/client/job_test.go b/client/job_test.go index 76141ab5..b3207d6d 100644 --- a/client/job_test.go +++ b/client/job_test.go @@ -62,6 +62,7 @@ func TestJobs(t *testing.T) { require.ErrorContains(t, err, "received state FAILED while waiting for async process") require.ErrorContains(t, err, "cfclient error (CF-UnprocessableEntity|10008): something went wrong") require.ErrorContains(t, err, "cfclient error (UnknownError|10001): unexpected error occurred") + require.ErrorContains(t, err, "some warning") return nil, nil }, }, From 96a14398950ce345dea080c72c3b2fb1d068bfd8 Mon Sep 17 00:00:00 2001 From: Shawn Neal Date: Fri, 14 Feb 2025 14:44:21 -0800 Subject: [PATCH 4/5] Remove dead code, errors are returns via the poll func --- client/job.go | 9 --------- 1 file changed, 9 deletions(-) diff --git a/client/job.go b/client/job.go index 5008acd1..857f9b64 100644 --- a/client/job.go +++ b/client/job.go @@ -2,7 +2,6 @@ package client import ( "context" - "github.com/cloudfoundry/go-cfclient/v3/internal/path" "github.com/cloudfoundry/go-cfclient/v3/resource" ) @@ -35,13 +34,5 @@ func (c *JobClient) PollComplete(ctx context.Context, jobGUID string, opts *Poll } return "", "", err }, string(resource.JobStateComplete), opts) - - // attempt to return the underlying saved job error - if err == AsyncProcessFailedError { - job, _ := c.Get(ctx, jobGUID) - if job != nil && len(job.Errors) > 0 { - return job.Errors[0] - } - } return err } From 39f5d34fd77c268b7871ad1e6b362be0c5f920f4 Mon Sep 17 00:00:00 2001 From: Shawn Neal Date: Fri, 14 Feb 2025 14:45:19 -0800 Subject: [PATCH 5/5] Add custom error message with dynamic failed state --- client/polling.go | 6 +++--- client/polling_test.go | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/client/polling.go b/client/polling.go index b55d585e..daf09c09 100644 --- a/client/polling.go +++ b/client/polling.go @@ -2,10 +2,10 @@ package client import ( "errors" + "fmt" "time" ) -var AsyncProcessFailedError = errors.New("received state FAILED while waiting for async process") var AsyncProcessTimeoutError = errors.New("timed out after waiting for async process") type PollingOptions struct { @@ -38,7 +38,7 @@ func PollForStateOrTimeout(getState getStateFunc, successState string, opts *Pol case <-timeout: return AsyncProcessTimeoutError case <-ticker.C: - state, cferror, err := getState() + state, cfError, err := getState() if err != nil { return err } @@ -46,7 +46,7 @@ func PollForStateOrTimeout(getState getStateFunc, successState string, opts *Pol case successState: return nil case opts.FailedState: - return errors.Join(AsyncProcessFailedError, errors.New(cferror)) + return fmt.Errorf("received state %s while waiting for async process: %s", opts.FailedState, cfError) } } } diff --git a/client/polling_test.go b/client/polling_test.go index be1e416d..2ac0ee9f 100644 --- a/client/polling_test.go +++ b/client/polling_test.go @@ -1,7 +1,6 @@ package client import ( - "errors" "testing" "time" @@ -33,7 +32,8 @@ func TestPollForStateOrTimeout(t *testing.T) { } err := PollForStateOrTimeout(failedFn, "NOPE", noWaitOpts) - require.Equal(t, errors.Join(AsyncProcessFailedError, errors.New(CustomStagingErr)), err) + require.Error(t, err) + require.Equal(t, "received state FAILED while waiting for async process: "+CustomStagingErr, err.Error()) err = PollForStateOrTimeout(successFn, "SUCCESS", noWaitOpts) require.NoError(t, err)