From e02074ab3f6fb9ac979be445bf852edec77739a0 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Thu, 7 Aug 2025 15:06:31 +0200 Subject: [PATCH 1/6] Better error diagnostic for SDK errors Takes advantage of https://github.com/databricks/databricks-sdk-go/pull/1261 --- .../resources/jobs/create-error/output.txt | 7 ++- bundle/terranova/tnresources/app.go | 4 +- bundle/terranova/tnresources/job.go | 14 +---- bundle/terranova/tnresources/pipeline.go | 14 +---- bundle/terranova/tnresources/schema.go | 10 +-- bundle/terranova/tnresources/sdk_error.go | 43 ------------- bundle/terranova/tnresources/sql_warehouse.go | 10 +-- bundle/terranova/tnresources/volume.go | 8 +-- libs/diag/diagnostic.go | 3 +- libs/diag/sdk_error.go | 61 +++++++++++++++++++ 10 files changed, 87 insertions(+), 87 deletions(-) delete mode 100644 bundle/terranova/tnresources/sdk_error.go create mode 100644 libs/diag/sdk_error.go diff --git a/acceptance/bundle/resources/jobs/create-error/output.txt b/acceptance/bundle/resources/jobs/create-error/output.txt index 10d49de6241..310772ff2ea 100644 --- a/acceptance/bundle/resources/jobs/create-error/output.txt +++ b/acceptance/bundle/resources/jobs/create-error/output.txt @@ -6,7 +6,12 @@ Warning: required field "new_cluster" is not set Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle/default/files... Deploying resources... -Error: deploying jobs.foo: creating: Method=Jobs.Create *retries.Err *apierr.APIError StatusCode=400 ErrorCode="INVALID_PARAMETER_VALUE" Message="Shared job cluster feature is only supported in multi-task jobs." +Error: deploying jobs.foo: creating: Shared job cluster feature is only supported in multi-task jobs. (400 INVALID_PARAMETER_VALUE) + +Endpoint: POST [DATABRICKS_URL]/api/2.2/jobs/create +HTTP Status: 400 Bad Request +API error_code: INVALID_PARAMETER_VALUE +API message: Shared job cluster feature is only supported in multi-task jobs. Updating deployment state... diff --git a/bundle/terranova/tnresources/app.go b/bundle/terranova/tnresources/app.go index 62ff1bc085a..5542243a8af 100644 --- a/bundle/terranova/tnresources/app.go +++ b/bundle/terranova/tnresources/app.go @@ -33,7 +33,7 @@ func (r *ResourceApp) DoCreate(ctx context.Context) (string, error) { } waiter, err := r.client.Apps.Create(ctx, request) if err != nil { - return "", SDKError{Method: "Apps.Create", Err: err} + return "", err } // TODO: Store waiter for Wait method @@ -48,7 +48,7 @@ func (r *ResourceApp) DoUpdate(ctx context.Context, id string) error { } response, err := r.client.Apps.Update(ctx, request) if err != nil { - return SDKError{Method: "Apps.Update", Err: err} + return err } if response.Name != id { diff --git a/bundle/terranova/tnresources/job.go b/bundle/terranova/tnresources/job.go index ab0002a2b52..efb69968f6c 100644 --- a/bundle/terranova/tnresources/job.go +++ b/bundle/terranova/tnresources/job.go @@ -33,7 +33,7 @@ func (r *ResourceJob) DoCreate(ctx context.Context) (string, error) { } response, err := r.client.Jobs.Create(ctx, request) if err != nil { - return "", SDKError{Method: "Jobs.Create", Err: err} + return "", err } return strconv.FormatInt(response.JobId, 10), nil } @@ -43,11 +43,7 @@ func (r *ResourceJob) DoUpdate(ctx context.Context, id string) error { if err != nil { return err } - err = r.client.Jobs.Reset(ctx, request) - if err != nil { - return SDKError{Method: "Jobs.Reset", Err: err} - } - return nil + return r.client.Jobs.Reset(ctx, request) } func DeleteJob(ctx context.Context, client *databricks.WorkspaceClient, id string) error { @@ -55,11 +51,7 @@ func DeleteJob(ctx context.Context, client *databricks.WorkspaceClient, id strin if err != nil { return err } - err = client.Jobs.DeleteByJobId(ctx, idInt) - if err != nil { - return SDKError{Method: "Jobs.DeleteByJobId", Err: err} - } - return nil + return client.Jobs.DeleteByJobId(ctx, idInt) } func (r *ResourceJob) WaitAfterCreate(ctx context.Context) error { diff --git a/bundle/terranova/tnresources/pipeline.go b/bundle/terranova/tnresources/pipeline.go index e1f81467510..3625d25c70d 100644 --- a/bundle/terranova/tnresources/pipeline.go +++ b/bundle/terranova/tnresources/pipeline.go @@ -27,7 +27,7 @@ func (r *ResourcePipeline) Config() any { func (r *ResourcePipeline) DoCreate(ctx context.Context) (string, error) { response, err := r.client.Pipelines.Create(ctx, r.config) if err != nil { - return "", SDKError{Method: "Pipelines.Create", Err: err} + return "", err } return response.PipelineId, nil } @@ -68,19 +68,11 @@ func (r *ResourcePipeline) DoUpdate(ctx context.Context, id string) error { ForceSendFields: filterFields[pipelines.EditPipeline](r.config.ForceSendFields), } - err := r.client.Pipelines.Update(ctx, request) - if err != nil { - return SDKError{Method: "Pipelines.Update", Err: err} - } - return nil + return r.client.Pipelines.Update(ctx, request) } func DeletePipeline(ctx context.Context, client *databricks.WorkspaceClient, id string) error { - err := client.Pipelines.DeleteByPipelineId(ctx, id) - if err != nil { - return SDKError{Method: "Pipelines.DeleteByPipelineId", Err: err} - } - return nil + return client.Pipelines.DeleteByPipelineId(ctx, id) } func (r *ResourcePipeline) WaitAfterCreate(ctx context.Context) error { diff --git a/bundle/terranova/tnresources/schema.go b/bundle/terranova/tnresources/schema.go index 3c362171fa4..f041dc77b63 100644 --- a/bundle/terranova/tnresources/schema.go +++ b/bundle/terranova/tnresources/schema.go @@ -28,7 +28,7 @@ func (r *ResourceSchema) Config() any { func (r *ResourceSchema) DoCreate(ctx context.Context) (string, error) { response, err := r.client.Schemas.Create(ctx, r.config) if err != nil { - return "", SDKError{Method: "Schemas.Create", Err: err} + return "", err } return response.FullName, nil } @@ -46,7 +46,7 @@ func (r *ResourceSchema) DoUpdate(ctx context.Context, id string) error { response, err := r.client.Schemas.Update(ctx, updateRequest) if err != nil { - return SDKError{Method: "Schemas.Update", Err: err} + return err } if response.FullName != id { @@ -57,15 +57,11 @@ func (r *ResourceSchema) DoUpdate(ctx context.Context, id string) error { } func DeleteSchema(ctx context.Context, client *databricks.WorkspaceClient, id string) error { - err := client.Schemas.Delete(ctx, catalog.DeleteSchemaRequest{ + return client.Schemas.Delete(ctx, catalog.DeleteSchemaRequest{ FullName: id, Force: true, ForceSendFields: nil, }) - if err != nil { - return SDKError{Method: "Schemas.Delete", Err: err} - } - return nil } func (r *ResourceSchema) WaitAfterCreate(ctx context.Context) error { diff --git a/bundle/terranova/tnresources/sdk_error.go b/bundle/terranova/tnresources/sdk_error.go deleted file mode 100644 index 82035de5226..00000000000 --- a/bundle/terranova/tnresources/sdk_error.go +++ /dev/null @@ -1,43 +0,0 @@ -package tnresources - -import ( - "fmt" - - "github.com/databricks/databricks-sdk-go/apierr" - "github.com/databricks/databricks-sdk-go/retries" -) - -// Default String() for APIError only returns Message. -// This wrapper adds other info from the response such as StatusCode, ErrorCode and type information. -// This is motivated by working with testserver which might have incomplete or broken error messages (empty Message). -// However it would not hurt to have this info when debugging real endpoints as well. -// It also adds Method which refers to API method that was called (e.g. Jobs.Reset). - -type SDKError struct { - Method string - Err error -} - -func (e SDKError) Error() string { - if e.Method != "" { - return fmt.Sprintf("Method=%s %s", e.Method, formatSDKError(e.Err)) - } else { - return formatSDKError(e.Err) - } -} - -func formatSDKError(e error) string { - retriesErr, ok := e.(*retries.Err) - if ok { - return fmt.Sprintf("%T %s", retriesErr, formatAPIError(retriesErr.Err)) - } - return formatAPIError(e) -} - -func formatAPIError(e error) string { - apiErr, ok := e.(*apierr.APIError) - if ok { - return fmt.Sprintf("%T StatusCode=%d ErrorCode=%#v Message=%#v", apiErr, apiErr.StatusCode, apiErr.ErrorCode, apiErr.Message) - } - return e.Error() -} diff --git a/bundle/terranova/tnresources/sql_warehouse.go b/bundle/terranova/tnresources/sql_warehouse.go index 440b72d6cb0..176d4f8ea0d 100644 --- a/bundle/terranova/tnresources/sql_warehouse.go +++ b/bundle/terranova/tnresources/sql_warehouse.go @@ -28,7 +28,7 @@ func (r *ResourceSqlWarehouse) Config() any { func (r *ResourceSqlWarehouse) DoCreate(ctx context.Context) (string, error) { waiter, err := r.client.Warehouses.Create(ctx, r.config) if err != nil { - return "", SDKError{Method: "Warehouses.Create", Err: err} + return "", err } return waiter.Id, nil @@ -55,7 +55,7 @@ func (r *ResourceSqlWarehouse) DoUpdate(ctx context.Context, id string) error { waiter, err := r.client.Warehouses.Edit(ctx, request) if err != nil { - return SDKError{Method: "Warehouses.Edit", Err: err} + return err } if waiter.Id != id { @@ -76,9 +76,5 @@ func (r *ResourceSqlWarehouse) WaitAfterUpdate(ctx context.Context) error { } func DeleteSqlWarehouse(ctx context.Context, client *databricks.WorkspaceClient, oldID string) error { - err := client.Warehouses.DeleteById(ctx, oldID) - if err != nil { - return SDKError{Method: "Warehouses.DeleteById", Err: err} - } - return nil + return client.Warehouses.DeleteById(ctx, oldID) } diff --git a/bundle/terranova/tnresources/volume.go b/bundle/terranova/tnresources/volume.go index e49cfc8e6e3..56d43052193 100644 --- a/bundle/terranova/tnresources/volume.go +++ b/bundle/terranova/tnresources/volume.go @@ -32,7 +32,7 @@ func (r *ResourceVolume) Config() any { func (r *ResourceVolume) DoCreate(ctx context.Context) (string, error) { response, err := r.client.Volumes.Create(ctx, r.config) if err != nil { - return "", SDKError{Method: "Volumes.Create", Err: err} + return "", err } return response.FullName, nil } @@ -58,7 +58,7 @@ func (r *ResourceVolume) DoUpdate(ctx context.Context, id string) error { response, err := r.client.Volumes.Update(ctx, updateRequest) if err != nil { - return SDKError{Method: "Volumes.Update", Err: err} + return err } if id != response.FullName { @@ -91,7 +91,7 @@ func (r *ResourceVolume) DoUpdateWithID(ctx context.Context, id string) (string, response, err := r.client.Volumes.Update(ctx, updateRequest) if err != nil { - return "", SDKError{Method: "Volumes.Update", Err: err} + return "", err } return response.FullName, nil @@ -100,7 +100,7 @@ func (r *ResourceVolume) DoUpdateWithID(ctx context.Context, id string) (string, func DeleteVolume(ctx context.Context, client *databricks.WorkspaceClient, id string) error { err := client.Volumes.DeleteByName(ctx, id) if err != nil { - return SDKError{Method: "Volumes.Delete", Err: err} + return err } return nil } diff --git a/libs/diag/diagnostic.go b/libs/diag/diagnostic.go index 0c7699b4ef5..98335a88b93 100644 --- a/libs/diag/diagnostic.go +++ b/libs/diag/diagnostic.go @@ -48,7 +48,8 @@ func FromErr(err error) Diagnostics { return []Diagnostic{ { Severity: Error, - Summary: err.Error(), + Summary: FormatAPIErrorSummary(err), + Detail: FormatAPIErrorDetails(err), }, } } diff --git a/libs/diag/sdk_error.go b/libs/diag/sdk_error.go new file mode 100644 index 00000000000..f42c2dbfa79 --- /dev/null +++ b/libs/diag/sdk_error.go @@ -0,0 +1,61 @@ +package diag + +import ( + "errors" + "fmt" + "strconv" + "strings" + + "github.com/databricks/databricks-sdk-go/apierr" +) + +func findApiErr(e error) *apierr.APIError { + for { + cast, ok := e.(*apierr.APIError) + if ok { + return cast + } + + inner := errors.Unwrap(e) + if inner == nil { + break + } + e = inner + } + return nil +} + +func FormatAPIErrorSummary(e error) string { + extra := "" + apiErr := findApiErr(e) + if apiErr != nil { + extra = strings.TrimSpace(fmt.Sprintf("%d %s", apiErr.StatusCode, apiErr.ErrorCode)) + extra = " (" + extra + ")" + } + + return e.Error() + extra +} + +func FormatAPIErrorDetails(e error) string { + apiErr := findApiErr(e) + if apiErr == nil { + return "" + } + endpoint := "n/a" + httpStatus := "" + w := apiErr.ResponseWrapper + if w != nil { + resp := w.Response + if resp != nil { + httpStatus = resp.Status + req := resp.Request + if req != nil { + endpoint = fmt.Sprintf("%s %s", req.Method, req.URL) + } + } + } + if len(httpStatus) == 0 { + httpStatus = strconv.Itoa(apiErr.StatusCode) + } + return fmt.Sprintf("Endpoint: %s\nHTTP Status: %s\nAPI error_code: %s\nAPI message: %s", endpoint, httpStatus, apiErr.ErrorCode, apiErr.Message) +} From 8f1b2cc63d161a3516405ac5579c73073a0d6f64 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Fri, 8 Aug 2025 11:34:38 +0200 Subject: [PATCH 2/6] rewrite to return early --- libs/diag/sdk_error.go | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/libs/diag/sdk_error.go b/libs/diag/sdk_error.go index f42c2dbfa79..682c20ad42b 100644 --- a/libs/diag/sdk_error.go +++ b/libs/diag/sdk_error.go @@ -26,14 +26,12 @@ func findApiErr(e error) *apierr.APIError { } func FormatAPIErrorSummary(e error) string { - extra := "" apiErr := findApiErr(e) - if apiErr != nil { - extra = strings.TrimSpace(fmt.Sprintf("%d %s", apiErr.StatusCode, apiErr.ErrorCode)) - extra = " (" + extra + ")" + if apiErr == nil { + return e.Error() } - - return e.Error() + extra + extra := strings.TrimSpace(fmt.Sprintf("%d %s", apiErr.StatusCode, apiErr.ErrorCode)) + return e.Error() + " (" + extra + ")" } func FormatAPIErrorDetails(e error) string { From 869a696e1f7c1fdceb6bd3bc636d20e0a470be4b Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Fri, 8 Aug 2025 11:39:35 +0200 Subject: [PATCH 3/6] use errors.As --- libs/diag/sdk_error.go | 25 +++++-------------------- 1 file changed, 5 insertions(+), 20 deletions(-) diff --git a/libs/diag/sdk_error.go b/libs/diag/sdk_error.go index 682c20ad42b..1099dd738dc 100644 --- a/libs/diag/sdk_error.go +++ b/libs/diag/sdk_error.go @@ -9,25 +9,9 @@ import ( "github.com/databricks/databricks-sdk-go/apierr" ) -func findApiErr(e error) *apierr.APIError { - for { - cast, ok := e.(*apierr.APIError) - if ok { - return cast - } - - inner := errors.Unwrap(e) - if inner == nil { - break - } - e = inner - } - return nil -} - func FormatAPIErrorSummary(e error) string { - apiErr := findApiErr(e) - if apiErr == nil { + var apiErr *apierr.APIError + if !errors.As(e, &apiErr) { return e.Error() } extra := strings.TrimSpace(fmt.Sprintf("%d %s", apiErr.StatusCode, apiErr.ErrorCode)) @@ -35,10 +19,11 @@ func FormatAPIErrorSummary(e error) string { } func FormatAPIErrorDetails(e error) string { - apiErr := findApiErr(e) - if apiErr == nil { + var apiErr *apierr.APIError + if !errors.As(e, &apiErr) { return "" } + endpoint := "n/a" httpStatus := "" w := apiErr.ResponseWrapper From a8f0088ccc1d94a041d7b38895c4760243d9c393 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Fri, 8 Aug 2025 12:12:11 +0200 Subject: [PATCH 4/6] add unit test --- libs/diag/sdk_error_test.go | 85 +++++++++++++++++++++++++++++++++++++ 1 file changed, 85 insertions(+) create mode 100644 libs/diag/sdk_error_test.go diff --git a/libs/diag/sdk_error_test.go b/libs/diag/sdk_error_test.go new file mode 100644 index 00000000000..6b51714bf87 --- /dev/null +++ b/libs/diag/sdk_error_test.go @@ -0,0 +1,85 @@ +package diag + +import ( + "errors" + "fmt" + "net/http" + "net/url" + "testing" + + "github.com/databricks/databricks-sdk-go/apierr" + "github.com/databricks/databricks-sdk-go/common" + "github.com/stretchr/testify/assert" +) + +func TestFormatAPIError(t *testing.T) { + u, _ := url.Parse("https://example.com/api/2.0/foo") + request := &http.Request{Method: http.MethodPost, URL: u} + + cases := []struct { + name string + err error + expectedSummary string + expectedDetails string + }{ + { + name: "plain error (no apierr)", + err: errors.New("foo"), + expectedSummary: "foo", + expectedDetails: "", + }, + { + name: "apierr no wrapper", + err: &apierr.APIError{ + Message: "api error message", + ErrorCode: "TEST_ERROR", + StatusCode: 400, + }, + expectedSummary: "api error message (400 TEST_ERROR)", + expectedDetails: "Endpoint: n/a\nHTTP Status: 400\nAPI error_code: TEST_ERROR\nAPI message: api error message", + }, + { + name: "apierr with Response only", + err: &apierr.APIError{ + Message: "api error message", + ErrorCode: "TEST_ERROR", + StatusCode: 400, + ResponseWrapper: &common.ResponseWrapper{ + Response: &http.Response{Status: "400 Bad Request"}, + }, + }, + expectedSummary: "api error message (400 TEST_ERROR)", + expectedDetails: "Endpoint: n/a\nHTTP Status: 400 Bad Request\nAPI error_code: TEST_ERROR\nAPI message: api error message", + }, + { + name: "apierr with full Response+Request", + err: &apierr.APIError{ + Message: "api error message", + ErrorCode: "TEST_ERROR", + StatusCode: 400, + ResponseWrapper: &common.ResponseWrapper{ + Response: &http.Response{Status: "400 Bad Request", Request: request}, + }, + }, + expectedSummary: "api error message (400 TEST_ERROR)", + expectedDetails: "Endpoint: POST https://example.com/api/2.0/foo\nHTTP Status: 400 Bad Request\nAPI error_code: TEST_ERROR\nAPI message: api error message", + }, + { + name: "wrapped apierr", + err: fmt.Errorf("wrapped: %w", &apierr.APIError{ + Message: "api error message", + ErrorCode: "TEST_ERROR", + StatusCode: 500, + }), + expectedSummary: "wrapped: api error message (500 TEST_ERROR)", + expectedDetails: "Endpoint: n/a\nHTTP Status: 500\nAPI error_code: TEST_ERROR\nAPI message: api error message", + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + assert.Equal(t, tc.expectedSummary, FormatAPIErrorSummary(tc.err)) + assert.Equal(t, tc.expectedDetails, FormatAPIErrorDetails(tc.err)) + }) + } +} From 7b9c3f648f37c486fd2fad011d8d129ff963e7ca Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Mon, 18 Aug 2025 11:21:57 +0200 Subject: [PATCH 5/6] update database_instance.go --- .../terranova/tnresources/database_instance.go | 18 ++++-------------- 1 file changed, 4 insertions(+), 14 deletions(-) diff --git a/bundle/terranova/tnresources/database_instance.go b/bundle/terranova/tnresources/database_instance.go index 9517fedf8b1..35bd2a183f6 100644 --- a/bundle/terranova/tnresources/database_instance.go +++ b/bundle/terranova/tnresources/database_instance.go @@ -23,7 +23,7 @@ func (d *ResourceDatabaseInstance) DoCreate(ctx context.Context) (string, error) DatabaseInstance: d.config, }) if err != nil { - return "", SDKError{Method: "Database.CreateDatabaseInstance", Err: err} + return "", err } d.waiter = waiter return waiter.Response.Name, nil @@ -38,10 +38,7 @@ func (d ResourceDatabaseInstance) DoUpdate(ctx context.Context, id string) error request.DatabaseInstance.Uid = id _, err := d.client.Database.UpdateDatabaseInstance(ctx, request) - if err != nil { - return SDKError{Method: "Database.UpdateDatabaseInstance", Err: err} - } - return nil + return err } func (d *ResourceDatabaseInstance) WaitAfterCreate(ctx context.Context) error { @@ -49,10 +46,7 @@ func (d *ResourceDatabaseInstance) WaitAfterCreate(ctx context.Context) error { return nil } _, err := d.waiter.Get() - if err != nil { - return SDKError{Method: "WaitGetDatabaseInstanceDatabaseAvailable.Get", Err: err} - } - return nil + return err } func (d ResourceDatabaseInstance) WaitAfterUpdate(ctx context.Context) error { @@ -68,14 +62,10 @@ func NewResourceDatabaseInstance(client *databricks.WorkspaceClient, resource *r } func DeleteDatabaseInstance(ctx context.Context, client *databricks.WorkspaceClient, name string) error { - err := client.Database.DeleteDatabaseInstance(ctx, database.DeleteDatabaseInstanceRequest{ + return client.Database.DeleteDatabaseInstance(ctx, database.DeleteDatabaseInstanceRequest{ Name: name, Purge: true, Force: false, ForceSendFields: nil, }) - if err != nil { - return SDKError{Method: "Database.DeleteDatabaseInstanceByName", Err: err} - } - return nil } From 31738625a84fe004dd19cc77835c461829fdf03e Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Mon, 18 Aug 2025 14:56:51 +0200 Subject: [PATCH 6/6] update NEXT_CHANGELOG.md --- NEXT_CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/NEXT_CHANGELOG.md b/NEXT_CHANGELOG.md index 138443e66f5..8919fad5a36 100644 --- a/NEXT_CHANGELOG.md +++ b/NEXT_CHANGELOG.md @@ -10,5 +10,6 @@ ### Bundles * Add support for Lakebase database instances in DABs ([#3283](https://github.com/databricks/cli/pull/3283)) +* Improve error message for SDK/API errors ([#3379](https://github.com/databricks/cli/pull/3379)) ### API Changes