-
Notifications
You must be signed in to change notification settings - Fork 24
Condense CatchV2ErrorDetailWithResponse and CatchV2ErrorMessageWithResponse into one #1406
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
055c05b
1e4238c
78a9b48
f3fbaff
d8cae1d
f9a9e9d
f7121ff
e8f31fc
24f3284
3d00280
f142d6c
a26bc75
4fe17f7
13918b0
f4b9084
937e3c7
86ae976
5a35f01
f0b8b2f
0f229ef
0584cb6
37eacbe
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -24,7 +24,7 @@ import ( | |
|
|
||
| const quotaExceededRegex = ".* is currently limited to .*" | ||
|
|
||
| type responseBody struct { | ||
| type errorResponseBody struct { | ||
| Error []errorDetail `json:"errors"` | ||
| Message string `json:"message"` | ||
| } | ||
|
|
@@ -141,26 +141,30 @@ func catchCCloudBackendUnmarshallingError(err error) error { | |
| CCLOUD-SDK-GO CLIENT ERROR CATCHING | ||
| */ | ||
|
|
||
| func CatchV2ErrorDetailWithResponse(err error, r *http.Response) error { | ||
| func CatchCCloudV2Error(err error, r *http.Response) error { | ||
| if err == nil { | ||
| return nil | ||
| } | ||
|
|
||
| if r == nil { | ||
| return err | ||
| } | ||
|
|
||
| body, _ := io.ReadAll(r.Body) | ||
| return CatchV2ErrorDetailWithResponseBody(err, body) | ||
| } | ||
|
|
||
| func CatchV2ErrorDetailWithResponseBody(err error, body []byte) error { | ||
| var resBody responseBody | ||
| var resBody errorResponseBody | ||
| _ = json.Unmarshal(body, &resBody) | ||
| if len(resBody.Error) > 0 { | ||
| detail := resBody.Error[0].Detail | ||
| if ok, _ := regexp.MatchString(quotaExceededRegex, detail); ok { | ||
| return NewWrapErrorWithSuggestions(err, detail, QuotaExceededSuggestions) | ||
|
Comment on lines
158
to
159
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not a huge fan that this is hardcoded here... this only applies to one service so it's wasteful to always check for it
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Which service? There are a few other specialized catchers in this file, maybe we can make a specialized one for this.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah... this probably needs an updating. I just took a look at some of the quotas by listing them on the CLI. Tried to create 11 environments and hit a quote error that matches So this hard-coding only prints that suggestion on whatever subset of commands uses (what is now) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yeah, it would be great if we could only wrap the quota service functions with this catcher There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this might be deserving of a separate pr
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I can make that my next cleanup task. I'm assuming only create and update subcommands can hit these quota errors. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We might introduce a new subcommand that also returns that error, so in my opinion the safest thing to do here would be to wrap every function in
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. New v2 requests to create a resource could result in this (429 if i remember correctly?).
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah ok, I didn't realize environment was using v1.
sgagniere marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } else if detail != "" { | ||
| return Wrap(err, strings.TrimSuffix(resBody.Error[0].Detail, "\n")) | ||
| return Wrap(err, strings.TrimSuffix(detail, "\n")) | ||
| } | ||
| } | ||
| if resBody.Message != "" { | ||
| return Wrap(err, strings.TrimRight(resBody.Message, "\n")) | ||
| } | ||
|
|
||
| return err | ||
| } | ||
|
|
||
|
|
@@ -184,10 +188,10 @@ func CatchEnvironmentNotFoundError(err error, r *http.Response) error { | |
| } | ||
|
|
||
| if r != nil && r.StatusCode == http.StatusForbidden { | ||
| return NewWrapErrorWithSuggestions(CatchV2ErrorDetailWithResponse(err, r), "environment not found or access forbidden", EnvNotFoundSuggestions) | ||
| return NewWrapErrorWithSuggestions(CatchCCloudV2Error(err, r), "environment not found or access forbidden", EnvNotFoundSuggestions) | ||
| } | ||
|
|
||
| return CatchV2ErrorDetailWithResponse(err, r) | ||
| return CatchCCloudV2Error(err, r) | ||
| } | ||
|
|
||
| func CatchKafkaNotFoundError(err error, clusterId string, r *http.Response) error { | ||
|
|
@@ -203,10 +207,10 @@ func CatchKafkaNotFoundError(err error, clusterId string, r *http.Response) erro | |
| if r.Request.Method == http.MethodDelete { | ||
| suggestions = KafkaClusterDeletingSuggestions | ||
| } | ||
| return NewWrapErrorWithSuggestions(CatchV2ErrorDetailWithResponse(err, r), "Kafka cluster not found or access forbidden", suggestions) | ||
| return NewWrapErrorWithSuggestions(CatchCCloudV2Error(err, r), "Kafka cluster not found or access forbidden", suggestions) | ||
| } | ||
|
|
||
| return CatchV2ErrorDetailWithResponse(err, r) | ||
| return CatchCCloudV2Error(err, r) | ||
| } | ||
|
|
||
| func CatchClusterConfigurationNotValidError(err error, r *http.Response) error { | ||
|
|
@@ -218,19 +222,19 @@ func CatchClusterConfigurationNotValidError(err error, r *http.Response) error { | |
| return err | ||
| } | ||
|
|
||
| body, _ := io.ReadAll(r.Body) | ||
| if strings.Contains(string(body), "CKU must be greater") { | ||
| err = CatchCCloudV2Error(err, r) | ||
| if strings.Contains(err.Error(), "CKU must be greater") { | ||
| return New(InvalidCkuErrorMsg) | ||
| } | ||
|
|
||
| return CatchV2ErrorDetailWithResponseBody(err, body) | ||
| return err | ||
| } | ||
|
|
||
| func CatchApiKeyForbiddenAccessError(err error, operation string, r *http.Response) error { | ||
| if r != nil && r.StatusCode == http.StatusForbidden || strings.Contains(err.Error(), "Unknown API key") { | ||
| return NewWrapErrorWithSuggestions(CatchV2ErrorDetailWithResponse(err, r), fmt.Sprintf("error %s API key", operation), APIKeyNotFoundSuggestions) | ||
| return NewWrapErrorWithSuggestions(CatchCCloudV2Error(err, r), fmt.Sprintf("error %s API key", operation), APIKeyNotFoundSuggestions) | ||
| } | ||
| return CatchV2ErrorDetailWithResponse(err, r) | ||
| return CatchCCloudV2Error(err, r) | ||
| } | ||
|
|
||
| func CatchKSQLNotFoundError(err error, clusterId string) error { | ||
|
|
@@ -253,13 +257,13 @@ func CatchServiceNameInUseError(err error, r *http.Response, serviceName string) | |
| return err | ||
| } | ||
|
|
||
| body, _ := io.ReadAll(r.Body) | ||
| if strings.Contains(string(body), "Service name is already in use") { | ||
| err = CatchCCloudV2Error(err, r) | ||
| if strings.Contains(err.Error(), "Service name is already in use") { | ||
| errorMsg := fmt.Sprintf(ServiceNameInUseErrorMsg, serviceName) | ||
| return NewErrorWithSuggestions(errorMsg, ServiceNameInUseSuggestions) | ||
| } | ||
|
|
||
| return CatchV2ErrorDetailWithResponseBody(err, body) | ||
| return err | ||
| } | ||
|
|
||
| func CatchServiceAccountNotFoundError(err error, r *http.Response, serviceAccountId string) error { | ||
|
|
@@ -273,31 +277,11 @@ func CatchServiceAccountNotFoundError(err error, r *http.Response, serviceAccoun | |
| errorMsg := fmt.Sprintf(ServiceAccountNotFoundErrorMsg, serviceAccountId) | ||
| return NewErrorWithSuggestions(errorMsg, ServiceAccountNotFoundSuggestions) | ||
| case http.StatusForbidden: | ||
| return NewWrapErrorWithSuggestions(CatchV2ErrorDetailWithResponse(err, r), "service account not found or access forbidden", ServiceAccountNotFoundSuggestions) | ||
| return NewWrapErrorWithSuggestions(CatchCCloudV2Error(err, r), "service account not found or access forbidden", ServiceAccountNotFoundSuggestions) | ||
| } | ||
| } | ||
|
|
||
| return CatchV2ErrorDetailWithResponse(err, r) | ||
| } | ||
|
|
||
| func CatchV2ErrorMessageWithResponse(err error, r *http.Response) error { | ||
| if err == nil { | ||
| return nil | ||
| } | ||
|
|
||
| if r == nil { | ||
| return err | ||
| } | ||
| body, _ := io.ReadAll(r.Body) | ||
| var resBody responseBody | ||
| _ = json.Unmarshal(body, &resBody) | ||
| if resBody.Message != "" { | ||
| // {"error_code":400,"message":"Connector configuration is invalid and contains 1 validation error(s). | ||
| // Errors: quickstart: Value \"CLICKM\" is not a valid \"Select a template\" type\n"} | ||
| return Wrap(err, strings.TrimSuffix(resBody.Message, "\n")) | ||
| } | ||
|
|
||
| return err | ||
| return CatchCCloudV2Error(err, r) | ||
| } | ||
|
|
||
| func isResourceNotFoundError(err error) bool { | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.