From 0563befe974ee985add80b35dae653b9265557be Mon Sep 17 00:00:00 2001 From: Ubuntu Date: Fri, 18 Jul 2025 13:45:07 +0000 Subject: [PATCH 1/2] Refactor error message for html answer --- client/client_test.go | 122 ++++++++++++++--------------------------- httpclient/response.go | 33 ++++++++++- 2 files changed, 73 insertions(+), 82 deletions(-) diff --git a/client/client_test.go b/client/client_test.go index 267635946..78133b8b0 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -7,15 +7,14 @@ import ( "io" "net/http" "net/url" - "runtime" "strings" "testing" "github.com/databricks/databricks-sdk-go/apierr" "github.com/databricks/databricks-sdk-go/config" + "github.com/databricks/databricks-sdk-go/httpclient" "github.com/databricks/databricks-sdk-go/internal/env" "github.com/databricks/databricks-sdk-go/useragent" - "github.com/databricks/databricks-sdk-go/version" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -353,11 +352,6 @@ func TestDoRemovesDoubleSlashesFromFilesAPI(t *testing.T) { } func TestNonJSONResponseIncludedInError(t *testing.T) { - cicdHeader := "" - if useragent.CiCdProvider() != "" { - cicdHeader = fmt.Sprintf(" cicd/%s", useragent.CiCdProvider()) - } - goVersion := strings.TrimPrefix(runtime.Version(), "go") type testCase struct { statusCode int status string @@ -365,55 +359,54 @@ func TestNonJSONResponseIncludedInError(t *testing.T) { } cases := []testCase{ { - statusCode: 400, - status: "Bad Request", - errorMessage: `failed to unmarshal response body: invalid character '<' looking for beginning of value. This is likely a bug in the Databricks SDK for Go or the underlying REST API. Please report this issue with the following debugging information to the SDK issue tracker at https://github.com/databricks/databricks-sdk-go/issues. Request log: -` + "```" + ` -GET /a -> * Host: -> * Accept: application/json -> * Authorization: REDACTED -> * User-Agent: unknown/0.0.0 databricks-sdk-go/` + version.Version + ` go/` + goVersion + ` os/` + runtime.GOOS + ` auth/pat` + cicdHeader + ` -< HTTP/2.0 Bad Request -< * Content-Type: text/html -< hello -` + "```", + statusCode: 400, + status: "Bad Request", + errorMessage: httpclient.ErrHTMLContent.Error(), }, { - statusCode: 500, - status: "Internal Server Error", - errorMessage: `failed to unmarshal response body: invalid character '<' looking for beginning of value. This is likely a bug in the Databricks SDK for Go or the underlying REST API. Please report this issue with the following debugging information to the SDK issue tracker at https://github.com/databricks/databricks-sdk-go/issues. Request log: -` + "```" + ` -GET /a -> * Host: -> * Accept: application/json -> * Authorization: REDACTED -> * User-Agent: unknown/0.0.0 databricks-sdk-go/` + version.Version + ` go/` + goVersion + ` os/` + runtime.GOOS + ` auth/pat` + cicdHeader + ` -< HTTP/2.0 Internal Server Error -< * Content-Type: text/html -< hello -` + "```", + statusCode: 500, + status: "Internal Server Error", + errorMessage: httpclient.ErrHTMLContent.Error(), }, { - statusCode: 200, - status: "OK", - errorMessage: `failed to unmarshal response body: invalid character '<' looking for beginning of value. This is likely a bug in the Databricks SDK for Go or the underlying REST API. Please report this issue with the following debugging information to the SDK issue tracker at https://github.com/databricks/databricks-sdk-go/issues. Request log: -` + "```" + ` -GET /a -> * Host: -> * Accept: application/json -> * Authorization: REDACTED -> * User-Agent: unknown/0.0.0 databricks-sdk-go/` + version.Version + ` go/` + goVersion + ` os/` + runtime.GOOS + ` auth/pat` + cicdHeader + ` -< HTTP/2.0 OK -< * Content-Type: text/html -< hello -` + "```", + statusCode: 200, + status: "OK", + errorMessage: httpclient.ErrHTMLContent.Error(), }, } + for _, tc := range cases { - tc := tc t.Run(fmt.Sprintf("%d %s", tc.statusCode, tc.status), func(t *testing.T) { - testNonJSONResponseIncludedInError(t, tc.statusCode, tc.status, tc.errorMessage) + c, err := New(&config.Config{ + Host: "some", + Token: "token", + ConfigFile: "/dev/null", + HTTPTransport: hc(func(r *http.Request) (*http.Response, error) { + r.Header.Del("traceparent") // clear nondeterministic traceparent header + return &http.Response{ + Proto: "HTTP/2.0", + Status: tc.status, + Body: io.NopCloser(strings.NewReader(`hello`)), + Request: r, + Header: http.Header{ + "Content-Type": []string{"text/html"}, + }, + }, nil + }), + }) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + var m map[string]string + gotErr := c.Do(context.Background(), "GET", "/a", nil, nil, nil, &m) + + if gotErr == nil { + t.Fatalf("expected error, got nil") + } + if !strings.Contains(gotErr.Error(), tc.errorMessage) { + t.Fatalf("expected error to contain %q, got %q", tc.errorMessage, gotErr.Error()) + } }) } } @@ -534,39 +527,6 @@ func TestUserAgentForCiCd(t *testing.T) { } -func testNonJSONResponseIncludedInError(t *testing.T, statusCode int, status, errorMessage string) { - c, err := New(&config.Config{ - Host: "some", - Token: "token", - ConfigFile: "/dev/null", - HTTPTransport: hc(func(r *http.Request) (*http.Response, error) { - // Clear traceparent header which is nondeterministic. - r.Header.Del("traceparent") - return &http.Response{ - Proto: "HTTP/2.0", - Status: status, - Body: io.NopCloser(strings.NewReader(`hello`)), - Request: r, - Header: http.Header{ - "Content-Type": []string{"text/html"}, - }, - }, nil - }), - }) - require.NoError(t, err) - var m map[string]string - err = c.Do( - context.Background(), - "GET", - "/a", - nil, - nil, - nil, - &m, - ) - require.EqualError(t, err, errorMessage) -} - func TestRetryOn503(t *testing.T) { var requested bool c, err := New(&config.Config{ diff --git a/httpclient/response.go b/httpclient/response.go index 2a07fe2cd..888f1b00c 100644 --- a/httpclient/response.go +++ b/httpclient/response.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "encoding/json" + "errors" "fmt" "io" "net/http" @@ -16,6 +17,29 @@ import ( "github.com/databricks/databricks-sdk-go/logger/httplog" ) +// ErrHTMLContent is returned when the response body is HTML instead of JSON. +// +// This almost always indicates an issue with your authentication configuration. +// If you encounter this error, please verify the following: +// +// - Databricks Host: Ensure your host is set correctly. +// - Permissions: Confirm that the authentication method has the required +// permissions for the API operation you are trying to perform. +// - Network/Proxy: If you are behind a corporate firewall, ensure it is not +// blocking or redirecting API traffic. +// +// A common cause of this error is Private Link redirecting the SDK to a login +// page, which the SDK cannot process. This usually happens when trying to +// access a Private Link-enabled workspace configured with no public internet +// access from a different network than the VPC endpoint belongs to. +// +// For more details, please refer to the [Unified Auth] documentation and +// [Private Link Authentication Troubleshooting]. +// +// [Unified Auth]: https://docs.databricks.com/aws/en/dev-tools/auth/unified-auth +// [Private Link Authentication Troubleshooting]: https://learn.microsoft.com/en-us/azure/databricks/security/network/classic/private-link-standard#authentication-troubleshooting +var ErrHTMLContent = errors.New("received HTML response instead of JSON") + func WithResponseHeader(key string, value *string) DoOption { return DoOption{ out: func(body *common.ResponseWrapper) error { @@ -91,7 +115,10 @@ func WithResponseUnmarshal(response any) DoOption { *bs = bodyBytes return nil } - if err = json.Unmarshal(bodyBytes, &response); err != nil { + if err := json.Unmarshal(bodyBytes, &response); err != nil { + if _, ok := err.(*json.SyntaxError); ok && isHTMLContent(bodyBytes) { + return ErrHTMLContent + } return fmt.Errorf("failed to unmarshal response body: %w. %s", err, makeUnexpectedResponse(body.Response, body.RequestBody.DebugBytes, bodyBytes)) } return nil @@ -99,6 +126,10 @@ func WithResponseUnmarshal(response any) DoOption { } } +func isHTMLContent(bodyBytes []byte) bool { + return strings.HasPrefix(string(bodyBytes), "<") +} + func findContentsField(response any) (*reflect.Value, bool) { value := reflect.ValueOf(response) value = reflect.Indirect(value) From d3de920c2d10afeb44f4be990e65cda036680dd9 Mon Sep 17 00:00:00 2001 From: Ubuntu Date: Tue, 22 Jul 2025 11:20:19 +0000 Subject: [PATCH 2/2] Add changelog --- NEXT_CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/NEXT_CHANGELOG.md b/NEXT_CHANGELOG.md index 247a88f94..2bf39db50 100644 --- a/NEXT_CHANGELOG.md +++ b/NEXT_CHANGELOG.md @@ -4,6 +4,9 @@ ### New Features and Improvements +- Add new error value `ErrHTMLContent` to differentiate parsing errors caused by + HTML content from other parsing errors. + ### Bug Fixes ### Documentation