Skip to content

Commit 5b6ea86

Browse files
fix(sagemaker): Improve SSH configuration error handling for SageMaker remote connections
1 parent 95216e4 commit 5b6ea86

File tree

5 files changed

+25
-19
lines changed

5 files changed

+25
-19
lines changed

packages/core/src/awsService/sagemaker/commands.ts

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -227,11 +227,9 @@ export async function openRemoteConnect(
227227
return await handleRunningSpaceWithEnabledAccess(node, ctx, spaceName)
228228
}
229229
} catch (err: any) {
230-
// Suppress errors that were already shown to user
231-
if (shouldSuppressError(err)) {
230+
if (isUserCancelledError(err)) {
232231
return
233232
}
234-
throw err
235233
}
236234
}
237235

@@ -355,8 +353,7 @@ async function handleRunningSpaceWithDisabledAccess(
355353
)
356354
await tryRemoteConnection(node, ctx, progress)
357355
} catch (err: any) {
358-
// Suppress errors that were already shown to user
359-
if (shouldSuppressError(err)) {
356+
if (isUserCancelledError(err)) {
360357
return
361358
}
362359
throw new ToolkitError(`Remote connection failed: ${err.message}`, {
@@ -402,8 +399,7 @@ async function handleStoppedSpace(
402399
}
403400
)
404401
} catch (err: any) {
405-
// Suppress errors that were already shown to user
406-
if (shouldSuppressError(err)) {
402+
if (isUserCancelledError(err)) {
407403
return
408404
}
409405
throw new ToolkitError(`Remote connection failed: ${(err as Error).message}`, {
@@ -436,8 +432,14 @@ async function handleRunningSpaceWithEnabledAccess(
436432
}
437433

438434
/**
439-
* Helper function to determine if an error should be suppressed (already shown to user)
435+
* Checks if an error represents a user cancellation or an error that was already shown to the user.
436+
*
437+
* These error codes indicate that an error dialog was already displayed, so we should
438+
* not re-throw or show additional error messages.
439+
*
440+
* @param err - The error to check
441+
* @returns true if the error was already shown to the user via a dialog or notification
440442
*/
441-
function shouldSuppressError(err: any): boolean {
443+
function isUserCancelledError(err: any): boolean {
442444
return err.code === SshConfigError || err.code === InstanceTypeError
443445
}

packages/core/src/awsService/sagemaker/constants.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,8 +47,8 @@ export const InstanceTypeNotSelectedMessage = (spaceName: string) => {
4747
export const RemoteAccessRequiredMessage =
4848
'This space requires remote access to be enabled.\nWould you like to restart the space and connect?\nAny unsaved work will be lost.'
4949

50-
export const SshConfigErrorMessage = (sshConfigPath: string) => {
51-
return `Unable to connect. ${sshConfigPath} contains SSH configuration syntax errors. Fix the errors to continue.`
50+
export const SshConfigErrorMessage = () => {
51+
return `Unable to connect. ~/.ssh/config contains invalid SSH configuration. Fix the errors to continue.`
5252
}
5353

5454
export const SmusDeeplinkSessionExpiredError = {

packages/core/src/awsService/sagemaker/model.ts

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
// Disabled: detached server files cannot import vscode.
77
/* eslint-disable no-restricted-imports */
88
import * as vscode from 'vscode'
9-
import { sshAgentSocketVariable, startSshAgent, startVscodeRemote } from '../../shared/extensions/ssh'
9+
import { getSshConfigPath, sshAgentSocketVariable, startSshAgent, startVscodeRemote } from '../../shared/extensions/ssh'
1010
import { createBoundProcess, ensureDependencies } from '../../shared/remoteSession'
1111
import { SshConfig } from '../../shared/sshConfig'
1212
import * as path from 'path'
@@ -99,14 +99,12 @@ export async function prepareDevEnvConnection(
9999
logger.error(`sagemaker: failed to add ssh config section: ${err.message}`)
100100

101101
if (err instanceof ToolkitError && err.code === 'SshCheckFailed') {
102-
const sshConfigPath = path.join(os.homedir(), '.ssh', 'config')
102+
const sshConfigPath = getSshConfigPath()
103103

104+
// Extracting line number from SSH error message is the best-effort.
105+
// SSH error formats are not standardized and may vary across implementations.
104106
await vscode.window
105-
.showErrorMessage(
106-
SshConfigErrorMessage(sshConfigPath),
107-
{ modal: true, detail: err.message },
108-
'Open SSH Config'
109-
)
107+
.showErrorMessage(SshConfigErrorMessage(), { modal: true, detail: err.message }, 'Open SSH Config')
110108
.then((resp) => {
111109
if (resp === 'Open SSH Config') {
112110
void vscode.window.showTextDocument(vscode.Uri.file(sshConfigPath))

packages/core/src/shared/sshConfig.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,9 +85,11 @@ export class SshConfig {
8585
protected async matchSshSection() {
8686
const result = await this.checkSshOnHost()
8787
if (result.exitCode !== 0) {
88-
// Format stderr error message
88+
// Format stderr error message for display to user
8989
let errorMessage = result.stderr?.trim() || `ssh check against host failed: ${result.exitCode}`
9090
const sshConfigPath = getSshConfigPath()
91+
// Remove the SSH config file path prefix from error messages to make them more readable
92+
// SSH errors often include the full path like "/Users/name/.ssh/config: line 5: Bad configuration option"
9193
errorMessage = errorMessage.replace(new RegExp(`${sshConfigPath}:\\s*`, 'g'), '').trim()
9294

9395
if (result.error) {
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
{
2+
"type": "Bug Fix",
3+
"description": "SageMaker: SSH configuration errors now display line numbers and include an \"Open SSH Config\" button"
4+
}

0 commit comments

Comments
 (0)