From 651f4c6fcd231d136703867fd53766b89b2871a4 Mon Sep 17 00:00:00 2001 From: Anton Nekipelov <226657+anton-107@users.noreply.github.com> Date: Wed, 2 Sep 2026 19:05:18 +0000 Subject: [PATCH] Split the SSH extension telemetry category by failure outcome CheckIDESSHExtension reports four distinct outcomes as the single IDE_SSH_EXTENSION_MISSING category: the --list-extensions call failing, a declined prompt, no way to ask for consent, and the install itself being rejected. That category is now the largest source of failed IDE-mode tunnels, and the four call for different fixes -- a list failure means the check never ran, while a rejected install points at the marketplace or a policy blocking it -- so one value cannot tell us which to fix. Give each outcome its own category, attributed through errors.Is sentinels rather than message matching. Only LIST_FAILED and INSTALL_FAILED are reachable under --auto-approve, which the VS Code extension's tunnel button always passes, so the split also separates button traffic from direct CLI use. IDE_SSH_EXTENSION_MISSING is retired rather than reused: rows written before this change still carry it, so a query spanning the release has to accept both. Co-authored-by: Isaac --- experimental/ssh/internal/client/client.go | 22 ++++++- .../internal/client/client_internal_test.go | 45 +++++++++++++ experimental/ssh/internal/vscode/run.go | 28 ++++++-- experimental/ssh/internal/vscode/run_test.go | 66 +++++++++++++++++++ libs/telemetry/protos/ssh_tunnel.go | 23 ++++++- 5 files changed, 174 insertions(+), 10 deletions(-) diff --git a/experimental/ssh/internal/client/client.go b/experimental/ssh/internal/client/client.go index 031bbd4d7af..fcf0fc8ce6f 100644 --- a/experimental/ssh/internal/client/client.go +++ b/experimental/ssh/internal/client/client.go @@ -298,7 +298,7 @@ func Run(ctx context.Context, client *databricks.WorkspaceClient, opts ClientOpt return err } if err := vscode.CheckIDESSHExtension(ctx, opts.IDE, opts.AutoApprove); err != nil { - outcome.errorCategory = protos.SshTunnelErrorCategoryIDESSHExtensionMissing + outcome.errorCategory = sshExtensionErrorCategory(err) return err } } @@ -1247,6 +1247,26 @@ type connectOutcome struct { err error } +// sshExtensionErrorCategory attributes a Remote SSH extension check failure to the outcome that +// caused it. The four are kept apart because they imply different fixes, and because only the +// first two can occur under --auto-approve, which the IDE button always passes -- so a shift +// between them and the consent outcomes distinguishes button traffic from direct CLI use. +func sshExtensionErrorCategory(err error) protos.SshTunnelErrorCategory { + switch { + case errors.Is(err, vscode.ErrSSHExtensionListFailed): + return protos.SshTunnelErrorCategoryIDESSHExtensionListFailed + case errors.Is(err, vscode.ErrSSHExtensionInstallFailed): + return protos.SshTunnelErrorCategoryIDESSHExtensionInstallFailed + case errors.Is(err, vscode.ErrSSHExtensionInstallDeclined): + return protos.SshTunnelErrorCategoryIDESSHExtensionInstallDeclined + case errors.Is(err, vscode.ErrSSHExtensionInstallUnavailable): + return protos.SshTunnelErrorCategoryIDESSHExtensionInstallUnavailable + } + // CheckIDESSHExtension wraps a sentinel on every failure path, so this is only reachable if + // a new one is added without a category. UNKNOWN keeps it countable; see category() below. + return protos.SshTunnelErrorCategoryUnknown +} + // category returns the error category to report. A cancelled context means the user // interrupted the attempt, whichever call happened to observe it first, so it wins over the // category recorded at the failure site. An unattributed failure is reported as UNKNOWN so diff --git a/experimental/ssh/internal/client/client_internal_test.go b/experimental/ssh/internal/client/client_internal_test.go index cb47cd71bb8..7f64774c783 100644 --- a/experimental/ssh/internal/client/client_internal_test.go +++ b/experimental/ssh/internal/client/client_internal_test.go @@ -12,6 +12,7 @@ import ( "time" "github.com/databricks/cli/experimental/ssh/internal/sshconfig" + "github.com/databricks/cli/experimental/ssh/internal/vscode" "github.com/databricks/cli/libs/cmdio" "github.com/databricks/cli/libs/telemetry/protos" "github.com/databricks/databricks-sdk-go/experimental/mocks" @@ -558,6 +559,50 @@ func TestConnectOutcomeCategory(t *testing.T) { } } +// The four Remote SSH extension outcomes were reported as one category until they were split, +// which left the largest IDE-mode failure bucket unattributable. Pin the mapping, including the +// wrapping, since CheckIDESSHExtension returns its sentinels wrapped in a message. +func TestSshExtensionErrorCategory(t *testing.T) { + tests := []struct { + name string + err error + want protos.SshTunnelErrorCategory + }{ + { + name: "list failure", + err: fmt.Errorf("%w in VS Code: %w", vscode.ErrSSHExtensionListFailed, errors.New("exit 4")), + want: protos.SshTunnelErrorCategoryIDESSHExtensionListFailed, + }, + { + name: "install failure", + err: fmt.Errorf("%w: %w", vscode.ErrSSHExtensionInstallFailed, errors.New("exit 3")), + want: protos.SshTunnelErrorCategoryIDESSHExtensionInstallFailed, + }, + { + name: "user declined the install", + err: fmt.Errorf("%w: install it with ...", vscode.ErrSSHExtensionInstallDeclined), + want: protos.SshTunnelErrorCategoryIDESSHExtensionInstallDeclined, + }, + { + name: "no way to ask for consent", + err: fmt.Errorf("%w: install it with ...", vscode.ErrSSHExtensionInstallUnavailable), + want: protos.SshTunnelErrorCategoryIDESSHExtensionInstallUnavailable, + }, + { + // Only reachable if a new failure path forgets its sentinel. + name: "unsentinelled failure falls back to UNKNOWN", + err: errors.New("something else"), + want: protos.SshTunnelErrorCategoryUnknown, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.want, sshExtensionErrorCategory(tt.err)) + }) + } +} + func TestBuildSshTunnelEventReportsErrorCategory(t *testing.T) { got := buildSshTunnelEvent(ClientOptions{ConnectionName: "my-conn", IDE: "vscode"}, connectOutcome{ errorCategory: protos.SshTunnelErrorCategoryIDECommandNotOnPath, diff --git a/experimental/ssh/internal/vscode/run.go b/experimental/ssh/internal/vscode/run.go index db52cff9f5d..08e66f64b02 100644 --- a/experimental/ssh/internal/vscode/run.go +++ b/experimental/ssh/internal/vscode/run.go @@ -2,6 +2,7 @@ package vscode import ( "context" + "errors" "fmt" "os" "os/exec" @@ -118,15 +119,28 @@ func isExtensionVersionAtLeast(version, minVersion string) bool { return semver.IsValid(v) && semver.Compare(v, "v"+minVersion) >= 0 } +// The ways CheckIDESSHExtension can fail. Callers match these with errors.Is to attribute a +// failure without matching on message text. They are separate because they call for different +// fixes: a list failure means the check never ran, an install failure points at the marketplace +// or a policy blocking it, and the two consent outcomes cannot happen under --auto-approve. +var ( + ErrSSHExtensionListFailed = errors.New("could not list installed extensions") + ErrSSHExtensionInstallFailed = errors.New("could not install the Remote SSH extension") + ErrSSHExtensionInstallDeclined = errors.New("install of the Remote SSH extension declined") + ErrSSHExtensionInstallUnavailable = errors.New("cannot prompt to install the Remote SSH extension") +) + // CheckIDESSHExtension verifies that the required Remote SSH extension is installed // with a compatible version, and offers to install/update it if not. // When autoApprove is true, the extension is installed without asking. +// +// Every returned error wraps one of the Err* sentinels above. func CheckIDESSHExtension(ctx context.Context, option string, autoApprove bool) error { ide := getIDE(option) out, err := exec.CommandContext(ctx, ide.Command, "--list-extensions", "--show-versions").Output() if err != nil { - return fmt.Errorf("failed to list %s extensions: %w", ide.Name, err) + return fmt.Errorf("%w in %s: %w", ErrSSHExtensionListFailed, ide.Name, err) } version, found := parseExtensionVersion(string(out), ide.SSHExtensionID) @@ -144,17 +158,17 @@ func CheckIDESSHExtension(ctx context.Context, option string, autoApprove bool) if !autoApprove { if !cmdio.IsPromptSupported(ctx) { - return fmt.Errorf("%s Install it with: %s --install-extension %s, or pass --auto-approve", - msg, ide.Command, ide.SSHExtensionID) + return fmt.Errorf("%w: %s Install it with: %s --install-extension %s, or pass --auto-approve", + ErrSSHExtensionInstallUnavailable, msg, ide.Command, ide.SSHExtensionID) } shouldInstall, err := cmdio.AskYesOrNo(ctx, msg+" Would you like to install it?") if err != nil { - return fmt.Errorf("failed to prompt user: %w", err) + return fmt.Errorf("%w: %w", ErrSSHExtensionInstallUnavailable, err) } if !shouldInstall { - return fmt.Errorf("%s Install it with: %s --install-extension %s", - msg, ide.Command, ide.SSHExtensionID) + return fmt.Errorf("%w: %s Install it with: %s --install-extension %s", + ErrSSHExtensionInstallDeclined, msg, ide.Command, ide.SSHExtensionID) } } else { cmdio.LogString(ctx, msg+" Installing automatically (--auto-approve).") @@ -165,7 +179,7 @@ func CheckIDESSHExtension(ctx context.Context, option string, autoApprove bool) installCmd.Stdout = os.Stdout installCmd.Stderr = os.Stderr if err := installCmd.Run(); err != nil { - return fmt.Errorf("failed to install extension %q: %w", ide.SSHExtensionName, err) + return fmt.Errorf("%w in %s: %w", ErrSSHExtensionInstallFailed, ide.Name, err) } return nil } diff --git a/experimental/ssh/internal/vscode/run_test.go b/experimental/ssh/internal/vscode/run_test.go index 2a33b7b3828..d17385a1ce4 100644 --- a/experimental/ssh/internal/vscode/run_test.go +++ b/experimental/ssh/internal/vscode/run_test.go @@ -232,6 +232,37 @@ func createFakeIDEExecutable(t *testing.T, dir, command, output string) { } } +// createFailingIDEExecutable writes a fake IDE command that exits non-zero for every +// invocation, so "--list-extensions" fails and the check never learns what is installed. +func createFailingIDEExecutable(t *testing.T, dir, command string) { + t.Helper() + if runtime.GOOS == "windows" { + err := os.WriteFile(filepath.Join(dir, command+".cmd"), []byte("@echo off\nexit /b 4\n"), 0o755) + require.NoError(t, err) + } else { + err := os.WriteFile(filepath.Join(dir, command), []byte("#!/bin/sh\nexit 4\n"), 0o755) + require.NoError(t, err) + } +} + +// createIDEExecutableFailingInstall writes a fake IDE command that lists extensions +// successfully but rejects "--install-extension", as a marketplace or policy block would. +func createIDEExecutableFailingInstall(t *testing.T, dir, command, output string) { + t.Helper() + if runtime.GOOS == "windows" { + payloadPath := filepath.Join(dir, command+"-payload.txt") + err := os.WriteFile(payloadPath, []byte(output), 0o644) + require.NoError(t, err) + script := fmt.Sprintf("@echo off\nif \"%%1\"==\"--install-extension\" exit /b 3\ntype \"%s\"\n", payloadPath) + err = os.WriteFile(filepath.Join(dir, command+".cmd"), []byte(script), 0o755) + require.NoError(t, err) + } else { + script := fmt.Sprintf("#!/bin/sh\nfor a in \"$@\"; do\n [ \"$a\" = \"--install-extension\" ] && exit 3\ndone\nprintf '%%s' '%s'\n", output) + err := os.WriteFile(filepath.Join(dir, command), []byte(script), 0o755) + require.NoError(t, err) + } +} + func TestCheckIDESSHExtension_UpToDate(t *testing.T) { tmpDir := t.TempDir() t.Setenv("PATH", tmpDir) @@ -268,6 +299,8 @@ func TestCheckIDESSHExtension_Missing(t *testing.T) { require.Error(t, err) assert.Contains(t, err.Error(), `"Remote - SSH"`) assert.Contains(t, err.Error(), "not installed") + // The test context is not a TTY, so consent cannot be asked for. + assert.ErrorIs(t, err, ErrSSHExtensionInstallUnavailable) } func TestCheckIDESSHExtension_Outdated(t *testing.T) { @@ -282,6 +315,7 @@ func TestCheckIDESSHExtension_Outdated(t *testing.T) { require.Error(t, err) assert.Contains(t, err.Error(), "0.100.0") assert.Contains(t, err.Error(), ">= 0.120.0") + assert.ErrorIs(t, err, ErrSSHExtensionInstallUnavailable) } func TestCheckIDESSHExtension_Cursor(t *testing.T) { @@ -319,4 +353,36 @@ func TestCheckIDESSHExtension_NoPrompt_WithoutAutoApprove_Errors(t *testing.T) { err := CheckIDESSHExtension(ctx, VSCodeOption, false) require.Error(t, err) assert.Contains(t, err.Error(), "--install-extension") + assert.ErrorIs(t, err, ErrSSHExtensionInstallUnavailable) +} + +// A command that is on PATH but whose --list-extensions fails is reported separately from a +// missing extension: nothing was learned about what is installed, so it is not an install +// problem. CheckIDECommand passes here, since the command does resolve. +func TestCheckIDESSHExtension_ListFails(t *testing.T) { + tmpDir := t.TempDir() + t.Setenv("PATH", tmpDir) + ctx, _ := cmdio.NewTestContextWithStdout(t.Context()) + + createFailingIDEExecutable(t, tmpDir, "code") + + err := CheckIDESSHExtension(ctx, VSCodeOption, true) + require.Error(t, err) + assert.ErrorIs(t, err, ErrSSHExtensionListFailed) + assert.NotErrorIs(t, err, ErrSSHExtensionInstallFailed) +} + +// With --auto-approve there is no prompt, so a missing extension goes straight to an install. +// A rejected install is the one outcome the IDE button can produce on this path. +func TestCheckIDESSHExtension_AutoApprove_InstallFails(t *testing.T) { + tmpDir := t.TempDir() + t.Setenv("PATH", tmpDir) + ctx, _ := cmdio.NewTestContextWithStdout(t.Context()) + + createIDEExecutableFailingInstall(t, tmpDir, "code", "ms-python.python@2024.1.1\n") + + err := CheckIDESSHExtension(ctx, VSCodeOption, true) + require.Error(t, err) + assert.ErrorIs(t, err, ErrSSHExtensionInstallFailed) + assert.NotErrorIs(t, err, ErrSSHExtensionInstallUnavailable) } diff --git a/libs/telemetry/protos/ssh_tunnel.go b/libs/telemetry/protos/ssh_tunnel.go index 1c9a5a9e539..76f737dad62 100644 --- a/libs/telemetry/protos/ssh_tunnel.go +++ b/libs/telemetry/protos/ssh_tunnel.go @@ -21,6 +21,10 @@ const ( // The categories name the distinct early-return sites of the connect flow so a failure can // be attributed without logging the error text, which carries cluster names, paths and user // names. +// +// IDE_SSH_EXTENSION_MISSING was retired in favour of the four IDE_SSH_EXTENSION_* categories +// below: it reported all four outcomes as one, and they call for different fixes. Rows written +// before the split still carry it, so a query spanning that release has to accept both. type SshTunnelErrorCategory string const ( @@ -30,8 +34,23 @@ const ( // condition rather than a transient failure, so it is distinguished from the rest. SshTunnelErrorCategoryIDECommandNotOnPath SshTunnelErrorCategory = "IDE_COMMAND_NOT_ON_PATH" - // The required Remote-SSH extension is missing or too old and was not installed. - SshTunnelErrorCategoryIDESSHExtensionMissing SshTunnelErrorCategory = "IDE_SSH_EXTENSION_MISSING" + // The IDE's installed-extension list could not be read, so whether the Remote SSH + // extension was present is unknown. Distinct from the install failures below because it + // says nothing about the extension itself, only that the check could not run. + SshTunnelErrorCategoryIDESSHExtensionListFailed SshTunnelErrorCategory = "IDE_SSH_EXTENSION_LIST_FAILED" + + // The Remote SSH extension was missing or too old, an install was attempted, and the IDE + // rejected it. Points at the marketplace or a policy that forbids the install rather than + // at anything the user chose. + SshTunnelErrorCategoryIDESSHExtensionInstallFailed SshTunnelErrorCategory = "IDE_SSH_EXTENSION_INSTALL_FAILED" + + // The user was asked to install the Remote SSH extension and declined. Unreachable with + // --auto-approve, so absent from IDE-button traffic. + SshTunnelErrorCategoryIDESSHExtensionInstallDeclined SshTunnelErrorCategory = "IDE_SSH_EXTENSION_INSTALL_DECLINED" + + // The Remote SSH extension was missing or too old and consent could not be obtained: no + // --auto-approve and no usable prompt. Also unreachable with --auto-approve. + SshTunnelErrorCategoryIDESSHExtensionInstallUnavailable SshTunnelErrorCategory = "IDE_SSH_EXTENSION_INSTALL_UNAVAILABLE" // IDE settings had to be updated for serverless but the update failed and the user // declined to continue (or --auto-approve turned the failure into an abort).