Skip to content
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.

Sign up for GitHub

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

Parse API Error messages with int error codes #960

Merged
merged 2 commits into from
Jul 3, 2024
+73 −44
Merged

Parse API Error messages with int error codes #960

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cb875e1
Parse int error code as string + test
renaudhartert-db Jul 2, 2024
079a0e4
Minor renaming
renaudhartert-db Jul 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 37 additions & 34 deletions apierr/errors.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,19 +20,6 @@ const (
errorInfoType string = "type.googleapis.com/google.rpc.ErrorInfo"
)

// APIErrorBody maps "proper" databricks rest api errors to a struct
type APIErrorBody struct {
ErrorCode string `json:"error_code,omitempty"`
Message string `json:"message,omitempty"`
Details []ErrorDetail `json:"details,omitempty"`
// The following two are for scim api only
// for RFC 7644 Section 3.7.3 https://tools.ietf.org/html/rfc7644#section-3.7.3
ScimDetail string `json:"detail,omitempty"`
ScimStatus string `json:"status,omitempty"`
ScimType string `json:"scimType,omitempty"`
API12Error string `json:"error,omitempty"`
}

type ErrorDetail struct {
Type string `json:"@type,omitempty"`
Reason string `json:"reason,omitempty"`
Expand Down Expand Up @@ -170,61 +157,77 @@ func GetAPIError(ctx context.Context, resp common.ResponseWrapper) error {

func parseErrorFromResponse(resp *http.Response, requestBody, responseBody []byte) *APIError {
if len(responseBody) == 0 {
return &APIError{
StatusCode: resp.StatusCode,
}
return &APIError{StatusCode: resp.StatusCode}
}

// Anonymous struct used to unmarshal JSON Databricks API error responses.
var errorBody struct {
ErrorCode any `json:"error_code,omitempty"` // int or string
Message string `json:"message,omitempty"`
Details []ErrorDetail `json:"details,omitempty"`
API12Error string `json:"error,omitempty"`

// The following fields are for scim api only. See RFC7644 section 3.7.3
// https://tools.ietf.org/html/rfc7644#section-3.7.3
ScimDetail string `json:"detail,omitempty"`
ScimStatus string `json:"status,omitempty"`
ScimType string `json:"scimType,omitempty"`
}
// try to read in nicely formatted API error response
var errorBody APIErrorBody
err := json.Unmarshal(responseBody, &errorBody)
if err != nil {
errorBody = parseUnknownError(resp, requestBody, responseBody, err)
if err := json.Unmarshal(responseBody, &errorBody); err != nil {
return unknownAPIError(resp, requestBody, responseBody, err)
}

// Convert API 1.2 error (which used a different format) to the new format.
if errorBody.API12Error != "" {
// API 1.2 has different response format, let's adapt
errorBody.Message = errorBody.API12Error
}
// Handle SCIM error message details

// Handle SCIM error message details.
if errorBody.Message == "" && errorBody.ScimDetail != "" {
if errorBody.ScimDetail == "null" {
errorBody.Message = "SCIM API Internal Error"
} else {
errorBody.Message = errorBody.ScimDetail
}
// add more context from SCIM responses
// Add more context from SCIM responses.
errorBody.Message = fmt.Sprintf("%s %s", errorBody.ScimType, errorBody.Message)
errorBody.Message = strings.Trim(errorBody.Message, " ")
errorBody.ErrorCode = fmt.Sprintf("SCIM_%s", errorBody.ScimStatus)
}

return &APIError{
Message: errorBody.Message,
ErrorCode: errorBody.ErrorCode,
ErrorCode: fmt.Sprintf("%v", errorBody.ErrorCode),
StatusCode: resp.StatusCode,
Details: errorBody.Details,
}
}

func parseUnknownError(resp *http.Response, requestBody, responseBody []byte, err error) (errorBody APIErrorBody) {
func unknownAPIError(resp *http.Response, requestBody, responseBody []byte, err error) *APIError {
apiErr := &APIError{
StatusCode: resp.StatusCode,
}

// this is most likely HTML... since un-marshalling JSON failed
// Status parts first in case html message is not as expected
statusParts := strings.SplitN(resp.Status, " ", 2)
if len(statusParts) < 2 {
errorBody.ErrorCode = "UNKNOWN"
apiErr.ErrorCode = "UNKNOWN"
} else {
errorBody.ErrorCode = strings.ReplaceAll(
strings.ToUpper(strings.Trim(statusParts[1], " .")),
" ", "_")
apiErr.ErrorCode = strings.ReplaceAll(strings.ToUpper(strings.Trim(statusParts[1], " .")), " ", "_")
}

stringBody := string(responseBody)
messageRE := regexp.MustCompile(`<pre>(.*)</pre>`)
messageMatches := messageRE.FindStringSubmatch(stringBody)
// No messages with <pre> </pre> format found so return a APIError
if len(messageMatches) < 2 {
errorBody.Message = MakeUnexpectedError(resp, err, requestBody, responseBody).Error()
return
apiErr.Message = MakeUnexpectedError(resp, err, requestBody, responseBody).Error()
} else {
apiErr.Message = strings.Trim(messageMatches[1], " .")
}
errorBody.Message = strings.Trim(messageMatches[1], " .")
return

return apiErr
}

func MakeUnexpectedError(resp *http.Response, err error, requestBody, responseBody []byte) error {
Expand Down
46 changes: 36 additions & 10 deletions apierr/errors_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import (
"github.com/stretchr/testify/assert"
)

func TestGetAPIErrorHandlesEmptyResponse(t *testing.T) {
func TestGetAPIError_handlesEmptyResponse(t *testing.T) {
resp := common.ResponseWrapper{
Response: &http.Response{
Request: &http.Request{
Expand All @@ -26,11 +26,13 @@ func TestGetAPIErrorHandlesEmptyResponse(t *testing.T) {
DebugBytes: []byte{},
ReadCloser: io.NopCloser(bytes.NewReader([]byte{})),
}

err := GetAPIError(context.Background(), resp)

assert.Equal(t, err.(*APIError).Message, "")
}

func TestGetAPIErrorAppliesOverrides(t *testing.T) {
func TestGetAPIError_appliesOverrides(t *testing.T) {
resp := common.ResponseWrapper{
Response: &http.Response{
StatusCode: http.StatusBadRequest,
Expand All @@ -44,20 +46,34 @@ func TestGetAPIErrorAppliesOverrides(t *testing.T) {
DebugBytes: []byte{},
ReadCloser: io.NopCloser(bytes.NewReader([]byte(`{"error_code": "INVALID_PARAMETER_VALUE", "message": "Cluster abc does not exist"}`))),
}
ctx := context.Background()
err := GetAPIError(ctx, resp)

err := GetAPIError(context.Background(), resp)

assert.ErrorIs(t, err, ErrResourceDoesNotExist)
}

func TestApiErrorTransientRegexMatches(t *testing.T) {
err := APIError{
Message: "worker env WorkerEnvId(workerenv-XXXXX) not found",
func TestGetAPIError_parseIntErrorCode(t *testing.T) {
resp := common.ResponseWrapper{
Response: &http.Response{
StatusCode: http.StatusBadRequest,
Request: &http.Request{
Method: "GET",
URL: &url.URL{
Path: "/api/2.0/clusters/get",
},
},
},
DebugBytes: []byte{},
ReadCloser: io.NopCloser(bytes.NewReader([]byte(`{"error_code": 500, "status_code": 400, "message": "Cluster abc does not exist"}`))),
}
ctx := context.Background()
assert.True(t, err.IsRetriable(ctx))

err := GetAPIError(context.Background(), resp)

assert.ErrorIs(t, err, ErrBadRequest)
assert.Equal(t, err.(*APIError).ErrorCode, "500")
}

func TestApiErrorMapsPrivateLinkRedirect(t *testing.T) {
func TestGetAPIError_mapsPrivateLinkRedirect(t *testing.T) {
resp := common.ResponseWrapper{
Response: &http.Response{
Request: &http.Request{
Expand All @@ -69,7 +85,17 @@ func TestApiErrorMapsPrivateLinkRedirect(t *testing.T) {
},
},
}

err := GetAPIError(context.Background(), resp)

assert.ErrorIs(t, err, ErrPermissionDenied)
assert.Equal(t, err.(*APIError).ErrorCode, "PRIVATE_LINK_VALIDATION_ERROR")
}

func TestAPIError_transientRegexMatches(t *testing.T) {
err := APIError{
Message: "worker env WorkerEnvId(workerenv-XXXXX) not found",
}

assert.True(t, err.IsRetriable(context.Background()))
}