Include ResponseWrapper in APIError#1261
Merged
Merged
Conversation
|
If integration tests don't run automatically, an authorized user can run them manually by following the instructions below: Trigger: Inputs:
Checks will be approved automatically on success. |
renaudhartert-db
approved these changes
Aug 7, 2025
denik
added a commit
to databricks/cli
that referenced
this pull request
Aug 7, 2025
Takes advantage of databricks/databricks-sdk-go#1261
denik
added a commit
to databricks/cli
that referenced
this pull request
Aug 7, 2025
Takes advantage of databricks/databricks-sdk-go#1261
Contributor
Author
|
Thanks! This is CLI integration: databricks/cli#3379 that shows how we going to use that: |
denik
added a commit
to databricks/cli
that referenced
this pull request
Aug 7, 2025
Takes advantage of databricks/databricks-sdk-go#1261
denik
added a commit
to databricks/cli
that referenced
this pull request
Aug 8, 2025
Takes advantage of databricks/databricks-sdk-go#1261
denik
added a commit
to databricks/cli
that referenced
this pull request
Aug 14, 2025
Takes advantage of databricks/databricks-sdk-go#1261
denik
added a commit
to databricks/cli
that referenced
this pull request
Aug 18, 2025
Takes advantage of databricks/databricks-sdk-go#1261
denik
added a commit
to databricks/cli
that referenced
this pull request
Aug 18, 2025
Takes advantage of databricks/databricks-sdk-go#1261
github-merge-queue Bot
pushed a commit
to databricks/cli
that referenced
this pull request
Aug 18, 2025
…iagnostics (#3379) ## Changes - Any API error passed to diag.FromErr or logdiag.LogError will automatically be formatted to include HTTP status/message, HTTP method + full url, API code and message. - Custom error from direct deployment is removed, it's no longer needed. ## Why SDK errors by default only contain "message" from payload which sometimes looks like this "Request 123 failed". DABs/CLI do not add a lot of context to errors, so that message is all you see. This makes debugging very hard. I've been fixing this in direct deployment by - adding context to errors everywhere - tagging errors with endpoint This PR makes tagging unnecessary and adds better error handling to all of CLI - as long as you use logdiag.LogError, it'll add those critical details, see create-error test for example of the output. Requires databricks/databricks-sdk-go#1261 ## Tests Existing tests.
denik
added a commit
to databricks/cli
that referenced
this pull request
May 20, 2026
…iagnostics (#3379) ## Changes - Any API error passed to diag.FromErr or logdiag.LogError will automatically be formatted to include HTTP status/message, HTTP method + full url, API code and message. - Custom error from direct deployment is removed, it's no longer needed. ## Why SDK errors by default only contain "message" from payload which sometimes looks like this "Request 123 failed". DABs/CLI do not add a lot of context to errors, so that message is all you see. This makes debugging very hard. I've been fixing this in direct deployment by - adding context to errors everywhere - tagging errors with endpoint This PR makes tagging unnecessary and adds better error handling to all of CLI - as long as you use logdiag.LogError, it'll add those critical details, see create-error test for example of the output. Requires databricks/databricks-sdk-go#1261 ## Tests Existing tests.
denik
pushed a commit
to databricks/cli
that referenced
this pull request
May 20, 2026
The reverse proxy in `libs/testproxy` re-marshalled `apierr.APIError`
into a `{error_code, message}` envelope, dropping `details[]` and any
other fields the workspace returned. As a result, acceptance tests run
against the cloud could not observe error metadata that real CLI/TF
invocations rely on.
This change forwards `apiErr.ResponseWrapper.DebugBytes` verbatim with
the original status code, so callers see exactly what the workspace
sent. As a knock-on fix, response headers in `includeResponseHeaders`
(e.g. `X-Databricks-Org-Id`) are now also passed through on the error
path — previously the `WithResponseHeader` visitors weren't invoked when
`apiClient.Do` returned an error.
`ResponseWrapper` has been populated on every `APIError` since
[databricks-sdk-go#1261](databricks/databricks-sdk-go#1261)
(v0.100.0); the CLI is on v0.132.0. A panic guards the invariant in case
the SDK ever changes shape.
bernardo-rodriguez
pushed a commit
to bernardo-rodriguez/b-cli
that referenced
this pull request
May 21, 2026
…ks#5263) The reverse proxy in `libs/testproxy` re-marshalled `apierr.APIError` into a `{error_code, message}` envelope, dropping `details[]` and any other fields the workspace returned. As a result, acceptance tests run against the cloud could not observe error metadata that real CLI/TF invocations rely on. This change forwards `apiErr.ResponseWrapper.DebugBytes` verbatim with the original status code, so callers see exactly what the workspace sent. As a knock-on fix, response headers in `includeResponseHeaders` (e.g. `X-Databricks-Org-Id`) are now also passed through on the error path — previously the `WithResponseHeader` visitors weren't invoked when `apiClient.Do` returned an error. `ResponseWrapper` has been populated on every `APIError` since [databricks-sdk-go#1261](databricks/databricks-sdk-go#1261) (v0.100.0); the CLI is on v0.132.0. A panic guards the invariant in case the SDK ever changes shape.
TanishqDatabricks
pushed a commit
to TanishqDatabricks/cli
that referenced
this pull request
May 22, 2026
…ks#5263) The reverse proxy in `libs/testproxy` re-marshalled `apierr.APIError` into a `{error_code, message}` envelope, dropping `details[]` and any other fields the workspace returned. As a result, acceptance tests run against the cloud could not observe error metadata that real CLI/TF invocations rely on. This change forwards `apiErr.ResponseWrapper.DebugBytes` verbatim with the original status code, so callers see exactly what the workspace sent. As a knock-on fix, response headers in `includeResponseHeaders` (e.g. `X-Databricks-Org-Id`) are now also passed through on the error path — previously the `WithResponseHeader` visitors weren't invoked when `apiClient.Do` returned an error. `ResponseWrapper` has been populated on every `APIError` since [databricks-sdk-go#1261](databricks/databricks-sdk-go#1261) (v0.100.0); the CLI is on v0.132.0. A panic guards the invariant in case the SDK ever changes shape.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changes are proposed in this pull request?
APIError includes ResponseWrapper field.
This allows for clients to generate their own error message that includes this critical bits:
For more verbose modes:
I plan to use it in CLI/DABs, currently I'm annotating errors with endpoint name, I'd be able to get rid of this boilerplate: https://github.com/databricks/cli/blob/0a7bd1a/bundle/terranova/tnresources/job.go#L34-L36
How is this tested?
Updated existing unit tests to (partially) compare this new field.