From 64fb3fcafba1f5f1200fde6fe938bd2e343bb460 Mon Sep 17 00:00:00 2001 From: Anton Nekipelov <226657+anton-107@users.noreply.github.com> Date: Thu, 5 Nov 2026 16:00:01 +0100 Subject: [PATCH 1/2] aitools: classify install errors by standard library error type Add error categories for aitools install failures and classify wrapped standard library errors (network, timeout, interrupt, missing manifest, permission and filesystem errors) by type, without reading error text. JSON decode errors stay UNCATEGORIZED. Co-authored-by: Isaac --- .../cli/aitools-install-error-categories.md | 1 + cmd/aitools/categorize.go | 60 ++++++++++++++++ cmd/aitools/categorize_test.go | 69 +++++++++++++++++++ libs/telemetry/protos/aitools_install.go | 28 ++++++++ 4 files changed, 158 insertions(+) create mode 100644 .nextchanges/cli/aitools-install-error-categories.md diff --git a/.nextchanges/cli/aitools-install-error-categories.md b/.nextchanges/cli/aitools-install-error-categories.md new file mode 100644 index 00000000000..1f1e51ba5ca --- /dev/null +++ b/.nextchanges/cli/aitools-install-error-categories.md @@ -0,0 +1 @@ +* `databricks aitools install --output json` now reports a more specific `error_category` (`MANIFEST_NOT_FOUND`, `PERMISSION_DENIED`, `FILESYSTEM`) for some failures it reported as `UNCATEGORIZED`. diff --git a/cmd/aitools/categorize.go b/cmd/aitools/categorize.go index b4ed1265f7a..fe5fd42f5a7 100644 --- a/cmd/aitools/categorize.go +++ b/cmd/aitools/categorize.go @@ -1,12 +1,22 @@ package aitools import ( + "context" "errors" + "io/fs" + "net" + "os" + "syscall" + "github.com/charmbracelet/huh" "github.com/databricks/cli/libs/aitools/installer" + "github.com/databricks/cli/libs/clicompat" "github.com/databricks/cli/libs/telemetry/protos" ) +// classifyInstallError maps an aitools error to its telemetry category without +// reading any error text. Typed installer errors win, then standard library +// error types. func classifyInstallError(err error) protos.AitoolsErrorCategory { if err == nil { return protos.AitoolsErrorCategoryUnspecified @@ -18,9 +28,59 @@ func classifyInstallError(err error) protos.AitoolsErrorCategory { if skill, ok := errors.AsType[*installer.SkillError](err); ok { return skillErrorCategory(skill) } + return stdlibErrorCategory(err) +} + +// stdlibErrorCategory classifies errors by their standard library type, which +// the installer wraps with %w: network and timeout errors from fetching the +// manifest or skill files, and filesystem errors from writing skills or the +// install state. JSON decode errors stay UNCATEGORIZED: the manifest, the +// release payload and the install state all decode JSON, so only typed +// installer errors can tell MANIFEST_INVALID from STATE_INVALID. +func stdlibErrorCategory(err error) protos.AitoolsErrorCategory { + switch { + case errors.Is(err, huh.ErrUserAborted): + return protos.AitoolsErrorCategoryUserAborted + case errors.Is(err, context.DeadlineExceeded): + return protos.AitoolsErrorCategoryTimeout + case errors.Is(err, context.Canceled): + return protos.AitoolsErrorCategoryCommandInterrupted + } + // I/O deadlines (os.ErrDeadlineExceeded) are net.Errors whose Timeout() + // is true. syscall.Errno also implements net.Error, so a bare OS error code + // (from a file operation) is not evidence of a network failure. + if netErr, ok := errors.AsType[net.Error](err); ok && !isErrno(netErr) { + if netErr.Timeout() { + return protos.AitoolsErrorCategoryTimeout + } + return protos.AitoolsErrorCategoryNetwork + } + if errors.Is(err, clicompat.ErrNotFound) { + return protos.AitoolsErrorCategoryManifestNotFound + } + if errors.Is(err, fs.ErrPermission) { + return protos.AitoolsErrorCategoryPermissionDenied + } + // The OS error wrappers: PathError (open, mkdir, ...), LinkError (rename) + // and SyscallError. Network errors that wrap a SyscallError returned above. + if isOSError[*fs.PathError](err) || isOSError[*os.LinkError](err) || isOSError[*os.SyscallError](err) { + return protos.AitoolsErrorCategoryFilesystem + } return protos.AitoolsErrorCategoryUncategorized } +func isOSError[T error](err error) bool { + _, ok := errors.AsType[T](err) + return ok +} + +// isErrno reports whether err itself, not something it wraps, is an Errno: +// network errors such as *net.OpError also wrap one. +func isErrno(err error) bool { + _, ok := err.(syscall.Errno) //nolint:errorlint // must not unwrap, see above + return ok +} + func skillErrorCategory(e *installer.SkillError) protos.AitoolsErrorCategory { switch e.Reason { case installer.ReasonSkillNotFound: diff --git a/cmd/aitools/categorize_test.go b/cmd/aitools/categorize_test.go index 03e6a94904b..0926697474b 100644 --- a/cmd/aitools/categorize_test.go +++ b/cmd/aitools/categorize_test.go @@ -1,13 +1,24 @@ package aitools import ( + "context" + "encoding/json" "errors" "fmt" + "io/fs" + "net" + "net/url" + "os" + "path/filepath" + "syscall" "testing" + "github.com/charmbracelet/huh" "github.com/databricks/cli/libs/aitools/installer" + "github.com/databricks/cli/libs/clicompat" "github.com/databricks/cli/libs/telemetry/protos" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestClassifyInstallError(t *testing.T) { @@ -73,3 +84,61 @@ func TestClassifyInstallError(t *testing.T) { }) } } + +func TestClassifyInstallErrorGenericCauses(t *testing.T) { + urlErr := &url.Error{Op: "Get", URL: "https://example.test", Err: errors.New("connection refused")} + cases := []struct { + name string + err error + want protos.AitoolsErrorCategory + }{ + {"network", fmt.Errorf("failed to fetch manifest: %w", urlErr), protos.AitoolsErrorCategoryNetwork}, + // The shape net/http returns for a refused connection: the Errno sits under the network types. + {"network errno", fmt.Errorf("failed to fetch manifest: %w", &url.Error{Op: "Get", URL: "https://example.test", Err: &net.OpError{Op: "dial", Net: "tcp", Err: os.NewSyscallError("connect", syscall.ECONNREFUSED)}}), protos.AitoolsErrorCategoryNetwork}, + // A sandbox that blocks outbound sockets: a network failure, even though the Errno is EACCES. + {"network permission denied", fmt.Errorf("failed to fetch manifest: %w", &url.Error{Op: "Get", URL: "https://example.test", Err: &net.OpError{Op: "dial", Net: "tcp", Err: os.NewSyscallError("connect", syscall.EACCES)}}), protos.AitoolsErrorCategoryNetwork}, + {"network timeout", fmt.Errorf("failed to fetch manifest: %w", &url.Error{Op: "Get", URL: "https://example.test", Err: os.ErrDeadlineExceeded}), protos.AitoolsErrorCategoryTimeout}, + {"deadline", fmt.Errorf("fetch: %w", context.DeadlineExceeded), protos.AitoolsErrorCategoryTimeout}, + {"cancelled", fmt.Errorf("fetch: %w", context.Canceled), protos.AitoolsErrorCategoryCommandInterrupted}, + {"cancelled request", &url.Error{Op: "Get", URL: "https://example.test", Err: context.Canceled}, protos.AitoolsErrorCategoryCommandInterrupted}, + {"manifest missing", fmt.Errorf("skills manifest at x@y: %w", clicompat.ErrNotFound), protos.AitoolsErrorCategoryManifestNotFound}, + // The manifest, release payload and install state all decode JSON; only typed errors can tell them apart. + {"json syntax error stays uncategorized", fmt.Errorf("failed to parse manifest: %w", json.Unmarshal([]byte(""), &struct{}{})), protos.AitoolsErrorCategoryUncategorized}, + {"json type error stays uncategorized", fmt.Errorf("failed to parse manifest: %w", json.Unmarshal([]byte(`{"a":1}`), &struct{ A string }{})), protos.AitoolsErrorCategoryUncategorized}, + {"permission", fmt.Errorf("write: %w", &fs.PathError{Op: "open", Path: "/x", Err: fs.ErrPermission}), protos.AitoolsErrorCategoryPermissionDenied}, + // syscall.Errno implements net.Error, so OS errors must not reach the network check. + {"permission errno", fmt.Errorf("write: %w", &fs.PathError{Op: "open", Path: "/x", Err: syscall.EACCES}), protos.AitoolsErrorCategoryPermissionDenied}, + {"permission on rename", fmt.Errorf("save state: %w", &os.LinkError{Op: "rename", Old: "/a", New: "/b", Err: syscall.EACCES}), protos.AitoolsErrorCategoryPermissionDenied}, + {"filesystem", fmt.Errorf("write: %w", &fs.PathError{Op: "mkdir", Path: "/x", Err: errors.New("read-only file system")}), protos.AitoolsErrorCategoryFilesystem}, + {"filesystem from os", fmt.Errorf("read state: %w", readMissingFile(t)), protos.AitoolsErrorCategoryFilesystem}, + {"filesystem on rename", fmt.Errorf("replace skill: %w", &os.LinkError{Op: "rename", Old: "/a", New: "/b", Err: syscall.EBUSY}), protos.AitoolsErrorCategoryFilesystem}, + {"filesystem syscall", fmt.Errorf("resolve cwd: %w", os.NewSyscallError("getwd", syscall.ENOENT)), protos.AitoolsErrorCategoryFilesystem}, + {"user abort", huh.ErrUserAborted, protos.AitoolsErrorCategoryUserAborted}, + {"plain text stays uncategorized", errors.New("something"), protos.AitoolsErrorCategoryUncategorized}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + assert.Equal(t, tc.want, classifyInstallError(tc.err)) + }) + } +} + +// A corrupt install state must not be blamed on the remote manifest. It stays +// UNCATEGORIZED until the installer returns a typed state error, which should +// change this to STATE_INVALID. +func TestClassifyInstallErrorCorruptStateIsUncategorized(t *testing.T) { + dir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(dir, ".state.json"), []byte("{not json"), 0o600)) + _, err := installer.LoadState(dir) + require.Error(t, err) + assert.Equal(t, protos.AitoolsErrorCategoryUncategorized, classifyInstallError(fmt.Errorf("failed to load install state: %w", err))) +} + +// readMissingFile returns the error the OS gives for a missing file, so the +// test covers the platform's own error value (syscall.ENOENT on Unix, +// ERROR_FILE_NOT_FOUND on Windows). +func readMissingFile(t *testing.T) error { + _, err := os.ReadFile(filepath.Join(t.TempDir(), "missing")) + require.Error(t, err) + return err +} diff --git a/libs/telemetry/protos/aitools_install.go b/libs/telemetry/protos/aitools_install.go index a90cc22d4df..c23503eea09 100644 --- a/libs/telemetry/protos/aitools_install.go +++ b/libs/telemetry/protos/aitools_install.go @@ -45,6 +45,34 @@ const ( AitoolsErrorCategoryUnsupportedScope AitoolsErrorCategory = "UNSUPPORTED_SCOPE" AitoolsErrorCategoryExperimentalSkill AitoolsErrorCategory = "EXPERIMENTAL_SKILL" AitoolsErrorCategoryUncategorized AitoolsErrorCategory = "UNCATEGORIZED" + + // Failures before any agent is touched. + AitoolsErrorCategoryInvalidFlags AitoolsErrorCategory = "INVALID_FLAGS" + AitoolsErrorCategoryUnknownAgent AitoolsErrorCategory = "UNKNOWN_AGENT" + AitoolsErrorCategoryPromptFailed AitoolsErrorCategory = "PROMPT_FAILED" + // The user aborted a prompt (Ctrl-C / Esc). + AitoolsErrorCategoryUserAborted AitoolsErrorCategory = "USER_ABORTED" + + // Fetching skills. The typed installer errors (release lookup, skills + // fetch) win over the generic network/timeout classification. + AitoolsErrorCategoryReleaseLookupFailed AitoolsErrorCategory = "RELEASE_LOOKUP_FAILED" + AitoolsErrorCategoryRateLimited AitoolsErrorCategory = "RATE_LIMITED" + AitoolsErrorCategorySkillsFetchFailed AitoolsErrorCategory = "SKILLS_FETCH_FAILED" + AitoolsErrorCategoryManifestNotFound AitoolsErrorCategory = "MANIFEST_NOT_FOUND" + AitoolsErrorCategoryManifestInvalid AitoolsErrorCategory = "MANIFEST_INVALID" + AitoolsErrorCategoryNetwork AitoolsErrorCategory = "NETWORK" + AitoolsErrorCategoryTimeout AitoolsErrorCategory = "TIMEOUT" + + // Local state and filesystem. + AitoolsErrorCategoryStateInvalid AitoolsErrorCategory = "STATE_INVALID" + AitoolsErrorCategoryPermissionDenied AitoolsErrorCategory = "PERMISSION_DENIED" + AitoolsErrorCategoryFilesystem AitoolsErrorCategory = "FILESYSTEM" + + // The agent's plugin CLI did not answer in time. + AitoolsErrorCategoryPluginCLITimeout AitoolsErrorCategory = "PLUGIN_CLI_TIMEOUT" + // The command's context was cancelled. SIGINT outside a prompt is not + // counted here: it ends the process before telemetry is uploaded. + AitoolsErrorCategoryCommandInterrupted AitoolsErrorCategory = "COMMAND_INTERRUPTED" ) // AitoolsAgentResult records one agent's failed or skipped outcome within an From a6fd3394f2989e10c37b594156938fa9fdecd369 Mon Sep 17 00:00:00 2001 From: Anton Nekipelov <226657+anton-107@users.noreply.github.com> Date: Thu, 5 Nov 2026 16:08:21 +0100 Subject: [PATCH 2/2] Add PR link to changelog fragment Co-authored-by: Isaac --- .nextchanges/cli/aitools-install-error-categories.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.nextchanges/cli/aitools-install-error-categories.md b/.nextchanges/cli/aitools-install-error-categories.md index 1f1e51ba5ca..dfbf2618a18 100644 --- a/.nextchanges/cli/aitools-install-error-categories.md +++ b/.nextchanges/cli/aitools-install-error-categories.md @@ -1 +1 @@ -* `databricks aitools install --output json` now reports a more specific `error_category` (`MANIFEST_NOT_FOUND`, `PERMISSION_DENIED`, `FILESYSTEM`) for some failures it reported as `UNCATEGORIZED`. +* `databricks aitools install --output json` now reports a more specific `error_category` (`MANIFEST_NOT_FOUND`, `PERMISSION_DENIED`, `FILESYSTEM`) for some failures it reported as `UNCATEGORIZED`. ([#7380](https://github.com/databricks/cli/pull/7380))