diff --git a/.nextchanges/cli/aitools-install-error-categories.md b/.nextchanges/cli/aitools-install-error-categories.md new file mode 100644 index 00000000000..dfbf2618a18 --- /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`. ([#7380](https://github.com/databricks/cli/pull/7380)) 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