From 17a3f56d950a54ba289e5af24f38093cb45e9b6b Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Mon, 8 Mar 2021 22:05:36 +0000 Subject: [PATCH 1/9] Handle wac template errors --- coder-sdk/error.go | 27 +++++++++++++++++++++++---- internal/cmd/envs.go | 35 ++++++++++++++++++++++++++++++++--- 2 files changed, 55 insertions(+), 7 deletions(-) diff --git a/coder-sdk/error.go b/coder-sdk/error.go index d0dbf447..b152514f 100644 --- a/coder-sdk/error.go +++ b/coder-sdk/error.go @@ -24,24 +24,43 @@ type apiError struct { // apiErrorMsg contains the rich error information returned by API errors. type apiErrorMsg struct { - Msg string `json:"msg"` + Msg string `json:"msg"` + Code string `json:"code"` + Details json.RawMessage `json:"details"` } // HTTPError represents an error from the Coder API. type HTTPError struct { *http.Response + cached *apiError + cachedErr error } -func (e *HTTPError) Error() string { +func (e *HTTPError) Payload() (*apiError, error) { var msg apiError + if e.cached != nil || e.cachedErr != nil { + return e.cached, e.cachedErr + } + // Try to decode the payload as an error, if it fails or if there is no error message, // return the response URL with the status. - if err := json.NewDecoder(e.Response.Body).Decode(&msg); err != nil || msg.Err.Msg == "" { + if err := json.NewDecoder(e.Response.Body).Decode(&msg); err != nil { + e.cachedErr = err + return nil, err + } + + e.cached = &msg + return &msg, nil +} + +func (e *HTTPError) Error() string { + apiErr, err := e.Payload() + if err != nil || apiErr.Err.Msg == "" { return fmt.Sprintf("%s: %d %s", e.Request.URL, e.StatusCode, e.Status) } // If the payload was a in the expected error format with a message, include it. - return msg.Err.Msg + return apiErr.Err.Msg } func bodyError(resp *http.Response) error { diff --git a/internal/cmd/envs.go b/internal/cmd/envs.go index aaa4b27e..280d37ac 100644 --- a/internal/cmd/envs.go +++ b/internal/cmd/envs.go @@ -290,8 +290,8 @@ func createEnvFromRepoCmd() *cobra.Command { Long: "Create a new Coder environment from a config file.", Hidden: true, Example: `# create a new environment from git repository template -coder envs create-from-repo --repo-url github.com/cdr/m --branch my-branch -coder envs create-from-repo -f coder.yaml`, +coder envs create-from-config --repo-url github.com/cdr/m --branch my-branch +coder envs create-from-config -f coder.yaml`, RunE: func(cmd *cobra.Command, args []string) error { ctx := cmd.Context() @@ -355,7 +355,7 @@ coder envs create-from-repo -f coder.yaml`, tpl, err := client.ParseTemplate(ctx, req) if err != nil { - return xerrors.Errorf("parse environment template config: %w", err) + return handleTemplateError(err) } provider, err := coderutil.DefaultWorkspaceProvider(ctx, client) @@ -397,6 +397,35 @@ coder envs create-from-repo -f coder.yaml`, return cmd } +func handleTemplateError(origError error) error { + var httpError *coder.HTTPError + if !xerrors.As(origError, &httpError) { + return origError + } + + ae, err := httpError.Payload() + if err != nil { + return origError + } + + switch ae.Err.Code { + case "wac_template": + type payload struct { + ErrorType string `json:"error_type"` + Msgs []string `json:"messages"` + } + + var p payload + err := json.Unmarshal(ae.Err.Details, &p) + if err != nil { + return origError + } + + return clog.Error(p.ErrorType, p.Msgs...) + } + return origError +} + func editEnvCmd() *cobra.Command { var ( org string From 377d5b2f17e778ef8042416ebd8853889115fc04 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Mon, 8 Mar 2021 22:17:18 +0000 Subject: [PATCH 2/9] Linting --- coder-sdk/error.go | 18 ++++++++++-------- internal/cmd/envs.go | 4 ++-- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/coder-sdk/error.go b/coder-sdk/error.go index b152514f..b3925953 100644 --- a/coder-sdk/error.go +++ b/coder-sdk/error.go @@ -17,13 +17,13 @@ var ErrPermissions = xerrors.New("insufficient permissions") // ErrAuthentication describes the error case in which the requester has invalid authentication. var ErrAuthentication = xerrors.New("invalid authentication") -// apiError is the expected payload format for our errors. -type apiError struct { - Err apiErrorMsg `json:"error"` +// APIError is the expected payload format for our errors. +type APIError struct { + Err APIErrorMsg `json:"error"` } -// apiErrorMsg contains the rich error information returned by API errors. -type apiErrorMsg struct { +// APIErrorMsg contains the rich error information returned by API errors. +type APIErrorMsg struct { Msg string `json:"msg"` Code string `json:"code"` Details json.RawMessage `json:"details"` @@ -32,12 +32,14 @@ type apiErrorMsg struct { // HTTPError represents an error from the Coder API. type HTTPError struct { *http.Response - cached *apiError + cached *APIError cachedErr error } -func (e *HTTPError) Payload() (*apiError, error) { - var msg apiError +// Payload decode the response body into the standard error structure. The `details` +// section is stored as a raw json, and type depends on the `code` field. +func (e *HTTPError) Payload() (*APIError, error) { + var msg APIError if e.cached != nil || e.cachedErr != nil { return e.cached, e.cachedErr } diff --git a/internal/cmd/envs.go b/internal/cmd/envs.go index 280d37ac..5119a7af 100644 --- a/internal/cmd/envs.go +++ b/internal/cmd/envs.go @@ -408,8 +408,8 @@ func handleTemplateError(origError error) error { return origError } - switch ae.Err.Code { - case "wac_template": + // TODO: Handle verbose case here too? + if ae.Err.Code == "wac_template" { type payload struct { ErrorType string `json:"error_type"` Msgs []string `json:"messages"` From 12ea48d624d272f7bf68387627520fba22198ef3 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Mon, 8 Mar 2021 22:25:20 +0000 Subject: [PATCH 3/9] Add explict comments --- internal/cmd/envs.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/internal/cmd/envs.go b/internal/cmd/envs.go index 5119a7af..68e36e2f 100644 --- a/internal/cmd/envs.go +++ b/internal/cmd/envs.go @@ -397,15 +397,16 @@ coder envs create-from-config -f coder.yaml`, return cmd } +// handleTemplateError attempts to convert the basic error into a more detailed clog error func handleTemplateError(origError error) error { var httpError *coder.HTTPError if !xerrors.As(origError, &httpError) { - return origError + return origError // Return the original } ae, err := httpError.Payload() if err != nil { - return origError + return origError // Return the original } // TODO: Handle verbose case here too? @@ -423,7 +424,8 @@ func handleTemplateError(origError error) error { return clog.Error(p.ErrorType, p.Msgs...) } - return origError + + return origError // Return the original } func editEnvCmd() *cobra.Command { From b0c83efece47bac8820569188b5515b9cce492ce Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Mon, 8 Mar 2021 22:34:12 +0000 Subject: [PATCH 4/9] linting: Thanks for the period reminder godot... --- internal/cmd/envs.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/cmd/envs.go b/internal/cmd/envs.go index 68e36e2f..fb685d87 100644 --- a/internal/cmd/envs.go +++ b/internal/cmd/envs.go @@ -397,7 +397,7 @@ coder envs create-from-config -f coder.yaml`, return cmd } -// handleTemplateError attempts to convert the basic error into a more detailed clog error +// handleTemplateError attempts to convert the basic error into a more detailed clog error. func handleTemplateError(origError error) error { var httpError *coder.HTTPError if !xerrors.As(origError, &httpError) { From 417cdbd5e9fdc1e592bd92f22e7dc7b4de1bf751 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Mon, 8 Mar 2021 22:40:18 +0000 Subject: [PATCH 5/9] Add verbose handling --- internal/cmd/envs.go | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/internal/cmd/envs.go b/internal/cmd/envs.go index fb685d87..28ccc0ae 100644 --- a/internal/cmd/envs.go +++ b/internal/cmd/envs.go @@ -410,19 +410,32 @@ func handleTemplateError(origError error) error { } // TODO: Handle verbose case here too? - if ae.Err.Code == "wac_template" { - type payload struct { + switch ae.Err.Code { + case "wac_template": + type templatePayload struct { ErrorType string `json:"error_type"` Msgs []string `json:"messages"` } - var p payload + var p templatePayload err := json.Unmarshal(ae.Err.Details, &p) if err != nil { return origError } return clog.Error(p.ErrorType, p.Msgs...) + case "verbose": + // TODO: We should move this to some general spot to decode this + type verbosePayload struct { + Verbose string `json:"verbose"` + } + var p verbosePayload + err := json.Unmarshal(ae.Err.Details, &p) + if err != nil { + return origError + } + + return clog.Error(origError.Error(), p.Verbose) } return origError // Return the original From 649b4689a17d83b57aaa128d0d4fb78f9ef21a43 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Mon, 8 Mar 2021 22:43:08 +0000 Subject: [PATCH 6/9] Handle verbose errors --- internal/cmd/envs.go | 48 ++------------------------------------ internal/cmd/errors.go | 52 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+), 46 deletions(-) create mode 100644 internal/cmd/errors.go diff --git a/internal/cmd/envs.go b/internal/cmd/envs.go index 28ccc0ae..8560748c 100644 --- a/internal/cmd/envs.go +++ b/internal/cmd/envs.go @@ -355,7 +355,7 @@ coder envs create-from-config -f coder.yaml`, tpl, err := client.ParseTemplate(ctx, req) if err != nil { - return handleTemplateError(err) + return handleAPIError(err) } provider, err := coderutil.DefaultWorkspaceProvider(ctx, client) @@ -370,7 +370,7 @@ coder envs create-from-config -f coder.yaml`, Namespace: provider.DefaultNamespace, }) if err != nil { - return xerrors.Errorf("create environment: %w", err) + return handleAPIError(err) } if follow { @@ -397,50 +397,6 @@ coder envs create-from-config -f coder.yaml`, return cmd } -// handleTemplateError attempts to convert the basic error into a more detailed clog error. -func handleTemplateError(origError error) error { - var httpError *coder.HTTPError - if !xerrors.As(origError, &httpError) { - return origError // Return the original - } - - ae, err := httpError.Payload() - if err != nil { - return origError // Return the original - } - - // TODO: Handle verbose case here too? - switch ae.Err.Code { - case "wac_template": - type templatePayload struct { - ErrorType string `json:"error_type"` - Msgs []string `json:"messages"` - } - - var p templatePayload - err := json.Unmarshal(ae.Err.Details, &p) - if err != nil { - return origError - } - - return clog.Error(p.ErrorType, p.Msgs...) - case "verbose": - // TODO: We should move this to some general spot to decode this - type verbosePayload struct { - Verbose string `json:"verbose"` - } - var p verbosePayload - err := json.Unmarshal(ae.Err.Details, &p) - if err != nil { - return origError - } - - return clog.Error(origError.Error(), p.Verbose) - } - - return origError // Return the original -} - func editEnvCmd() *cobra.Command { var ( org string diff --git a/internal/cmd/errors.go b/internal/cmd/errors.go new file mode 100644 index 00000000..e2b3672b --- /dev/null +++ b/internal/cmd/errors.go @@ -0,0 +1,52 @@ +package cmd + +import ( + "encoding/json" + + "cdr.dev/coder-cli/coder-sdk" + "cdr.dev/coder-cli/pkg/clog" + "golang.org/x/xerrors" +) + +// handleAPIError attempts to convert an api error into a more detailed clog error. +// If it cannot, it will return the original error +func handleAPIError(origError error) error { + var httpError *coder.HTTPError + if !xerrors.As(origError, &httpError) { + return origError // Return the original + } + + ae, err := httpError.Payload() + if err != nil { + return origError // Return the original + } + + switch ae.Err.Code { + case "wac_template": // template parse errors + type templatePayload struct { + ErrorType string `json:"error_type"` + Msgs []string `json:"messages"` + } + + var p templatePayload + err := json.Unmarshal(ae.Err.Details, &p) + if err != nil { + return origError + } + + return clog.Error(p.ErrorType, p.Msgs...) + case "verbose": + type verbosePayload struct { + Verbose string `json:"verbose"` + } + var p verbosePayload + err := json.Unmarshal(ae.Err.Details, &p) + if err != nil { + return origError + } + + return clog.Error(origError.Error(), p.Verbose) + } + + return origError // Return the original +} From 463a8827a935aada4210fc50de3a6ea82b965547 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Mon, 8 Mar 2021 22:48:03 +0000 Subject: [PATCH 7/9] godot will be the bane of me --- internal/cmd/errors.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/internal/cmd/errors.go b/internal/cmd/errors.go index e2b3672b..695a3e28 100644 --- a/internal/cmd/errors.go +++ b/internal/cmd/errors.go @@ -3,13 +3,14 @@ package cmd import ( "encoding/json" + "golang.org/x/xerrors" + "cdr.dev/coder-cli/coder-sdk" "cdr.dev/coder-cli/pkg/clog" - "golang.org/x/xerrors" ) // handleAPIError attempts to convert an api error into a more detailed clog error. -// If it cannot, it will return the original error +// If it cannot, it will return the original error. func handleAPIError(origError error) error { var httpError *coder.HTTPError if !xerrors.As(origError, &httpError) { From d5adf70c433b33cdaebdc086b92c523d1d37a1e4 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Mon, 8 Mar 2021 23:48:52 +0000 Subject: [PATCH 8/9] Rephrase from code review suggestion --- coder-sdk/error.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/coder-sdk/error.go b/coder-sdk/error.go index b3925953..2973046a 100644 --- a/coder-sdk/error.go +++ b/coder-sdk/error.go @@ -17,7 +17,7 @@ var ErrPermissions = xerrors.New("insufficient permissions") // ErrAuthentication describes the error case in which the requester has invalid authentication. var ErrAuthentication = xerrors.New("invalid authentication") -// APIError is the expected payload format for our errors. +// APIError is the expected payload format for API errors. type APIError struct { Err APIErrorMsg `json:"error"` } From fe5a60cbcaa95461c538390bdc377c1432e9621a Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Tue, 9 Mar 2021 17:24:38 +0000 Subject: [PATCH 9/9] Handle precondition errors --- internal/cmd/errors.go | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/internal/cmd/errors.go b/internal/cmd/errors.go index 695a3e28..e202037e 100644 --- a/internal/cmd/errors.go +++ b/internal/cmd/errors.go @@ -2,6 +2,7 @@ package cmd import ( "encoding/json" + "fmt" "golang.org/x/xerrors" @@ -47,6 +48,23 @@ func handleAPIError(origError error) error { } return clog.Error(origError.Error(), p.Verbose) + case "precondition": + type preconditionPayload struct { + Error string `json:"error"` + Message string `json:"message"` + Solution string `json:"solution"` + } + + var p preconditionPayload + err := json.Unmarshal(ae.Err.Details, &p) + if err != nil { + return origError + } + + return clog.Error(fmt.Sprintf("Precondition Error : Status Code=%d", httpError.StatusCode), + p.Message, + clog.BlankLine, + clog.Tipf(p.Solution)) } return origError // Return the original