From 4ecba0e545ee8f03509f7f78f70a762cd1022516 Mon Sep 17 00:00:00 2001 From: Sebastian Choren Date: Wed, 28 Aug 2024 18:57:04 -0300 Subject: [PATCH 1/3] fix: correctly display general errors --- cli/cloud/runner/multifile_orchestrator.go | 29 +--------------------- cli/cmd/resource_run_cmd.go | 8 +++++- cli/runner/orchestrator.go | 24 +++++++++++++++--- 3 files changed, 29 insertions(+), 32 deletions(-) diff --git a/cli/cloud/runner/multifile_orchestrator.go b/cli/cloud/runner/multifile_orchestrator.go index a526e0bfd4..f8a01d18fd 100644 --- a/cli/cloud/runner/multifile_orchestrator.go +++ b/cli/cloud/runner/multifile_orchestrator.go @@ -4,8 +4,6 @@ import ( "context" "errors" "fmt" - "io" - "net/http" "os" "sync" @@ -15,6 +13,7 @@ import ( "github.com/kubeshop/tracetest/cli/pkg/fileutil" "github.com/kubeshop/tracetest/cli/pkg/resourcemanager" "github.com/kubeshop/tracetest/cli/runner" + "github.com/kubeshop/tracetest/cli/varset" "github.com/kubeshop/tracetest/server/pkg/id" "go.uber.org/zap" @@ -357,29 +356,3 @@ func getRequiredGates(gates []string) []openapi.SupportedGates { return requiredGates } - -// HandleRunError handles errors returned by the server when running a test. -// It normalizes the handling of general errors, like 404, -// but more importantly, it processes the missing environment variables error -// so the orchestrator can request them from the user. -func HandleRunError(resp *http.Response, reqErr error) error { - body, err := io.ReadAll(resp.Body) - if err != nil { - return fmt.Errorf("could not read response body: %w", err) - } - resp.Body.Close() - - if resp.StatusCode == http.StatusNotFound { - return fmt.Errorf("resource not found in server") - } - - if resp.StatusCode == http.StatusUnprocessableEntity { - return varset.BuildMissingVarsError(body) - } - - if reqErr != nil { - return fmt.Errorf("could not run test suite: %w", reqErr) - } - - return nil -} diff --git a/cli/cmd/resource_run_cmd.go b/cli/cmd/resource_run_cmd.go index d82c760169..0ef03c6b0e 100644 --- a/cli/cmd/resource_run_cmd.go +++ b/cli/cmd/resource_run_cmd.go @@ -13,6 +13,7 @@ import ( "github.com/spf13/cobra" cloudCmd "github.com/kubeshop/tracetest/cli/cloud/cmd" + cliRunner "github.com/kubeshop/tracetest/cli/runner" ) var ( @@ -109,7 +110,12 @@ func runMultipleFiles(ctx context.Context) (string, error) { } exitCode, err := cloudCmd.RunMultipleFiles(ctx, cliLogger, httpClient, runParams, &cliConfig, runnerRegistry, output) - ExitCLI(exitCode) + // General Error is 1, which is the default for errors. if this is the case, + // let the CLI handle the error and exit. + // otherwise exit with the exit code. + if exitCode > cliRunner.ExitCodeGeneralError { + ExitCLI(exitCode) + } return "", err } diff --git a/cli/runner/orchestrator.go b/cli/runner/orchestrator.go index 38a7c4315a..b0e3ffbd1c 100644 --- a/cli/runner/orchestrator.go +++ b/cli/runner/orchestrator.go @@ -296,8 +296,6 @@ func (a orchestrator) writeJUnitReport(ctx context.Context, r Runner, result Run return err } -var source = "cli" - func getRequiredGates(gates []string) []openapi.SupportedGates { if len(gates) == 0 { return nil @@ -330,9 +328,29 @@ func HandleRunError(resp *http.Response, reqErr error) error { return varset.BuildMissingVarsError(body) } + if ok, msg := attemptToParseStructuredError(body); ok { + return fmt.Errorf("could not run resouce: %s", msg) + } + if reqErr != nil { - return fmt.Errorf("could not run test suite: %w", reqErr) + return fmt.Errorf("could not run resouce: %w", reqErr) } return nil } + +func attemptToParseStructuredError(body []byte) (bool, string) { + var parsed struct { + Status int `json:"status"` + Detail string `json:"detail"` + } + + err := jsonFormat.Unmarshal(body, &parsed) + if err != nil { + return false, "" + } + + msg := fmt.Sprintf("%s (code %d)", parsed.Detail, parsed.Status) + + return true, msg +} From 643aa1808572ab4e83d4e057e8b6854b99c489c1 Mon Sep 17 00:00:00 2001 From: Sebastian Choren Date: Wed, 28 Aug 2024 19:04:37 -0300 Subject: [PATCH 2/3] fix --- cli/runner/orchestrator.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cli/runner/orchestrator.go b/cli/runner/orchestrator.go index b0e3ffbd1c..5588329a4d 100644 --- a/cli/runner/orchestrator.go +++ b/cli/runner/orchestrator.go @@ -329,6 +329,7 @@ func HandleRunError(resp *http.Response, reqErr error) error { } if ok, msg := attemptToParseStructuredError(body); ok { + spew.Dump(body) return fmt.Errorf("could not run resouce: %s", msg) } @@ -346,7 +347,7 @@ func attemptToParseStructuredError(body []byte) (bool, string) { } err := jsonFormat.Unmarshal(body, &parsed) - if err != nil { + if err != nil || parsed.Status == 0 { return false, "" } From b3210195f756ba0a2e046c20ac84689916ec8246 Mon Sep 17 00:00:00 2001 From: Sebastian Choren Date: Wed, 28 Aug 2024 19:20:57 -0300 Subject: [PATCH 3/3] fixes --- cli/cloud/runner/multifile_orchestrator.go | 2 +- cli/runner/orchestrator.go | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/cli/cloud/runner/multifile_orchestrator.go b/cli/cloud/runner/multifile_orchestrator.go index f8a01d18fd..f81a4db7ec 100644 --- a/cli/cloud/runner/multifile_orchestrator.go +++ b/cli/cloud/runner/multifile_orchestrator.go @@ -308,7 +308,7 @@ func (o orchestrator) createRun(ctx context.Context, resource any, vars *varset. } if !errors.Is(err, varset.MissingVarsError{}) { // actual error, return - return result, resource, fmt.Errorf("cannot run test: %w", err) + return result, resource, fmt.Errorf("cannot create test run: %w", err) } // missing vars error diff --git a/cli/runner/orchestrator.go b/cli/runner/orchestrator.go index 5588329a4d..3d75dd4004 100644 --- a/cli/runner/orchestrator.go +++ b/cli/runner/orchestrator.go @@ -329,7 +329,6 @@ func HandleRunError(resp *http.Response, reqErr error) error { } if ok, msg := attemptToParseStructuredError(body); ok { - spew.Dump(body) return fmt.Errorf("could not run resouce: %s", msg) }