Skip to content

Commit 0605975

Browse files
fix(sagemaker): improve SSH configuration error handling for SageMakare remote connections (#8328)
## Problem When attempting to connect to a SageMaker Space, if the user's SSH config has syntax errors (even outside the sm_* section), users would see a cryptic error message: ```ssh check against host failed: 255``` This error message: Didn't explain what was wrong Didn't indicate it was an SSH config issue Didn't provide a way to fix the problem ## Solution Enhanced the SSH configuration error handling to provide a better user experience: Shows a clear message explaining the issue. Displays the actual SSH error (including line numbers) Provides an "Open SSH Config" button to directly open the config file for editing https://github.com/user-attachments/assets/ef4f5a0f-0525-4e4c-9e96-a2081997dd81 --- - Treat all work as PUBLIC. Private `feature/x` branches will not be squash-merged at release time. - Your code changes must meet the guidelines in [CONTRIBUTING.md](https://github.com/aws/aws-toolkit-vscode/blob/master/CONTRIBUTING.md#guidelines). - License: I confirm that my contribution is made under the terms of the Apache 2.0 license.
1 parent 94bd3fd commit 0605975

File tree

6 files changed

+94
-20
lines changed

6 files changed

+94
-20
lines changed

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

Lines changed: 30 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -17,12 +17,11 @@ import { prepareDevEnvConnection, tryRemoteConnection } from './model'
1717
import { ExtContext } from '../../shared/extensions'
1818
import { SagemakerClient } from '../../shared/clients/sagemaker'
1919
import { AccessDeniedException } from '@amzn/sagemaker-client'
20-
import { ToolkitError } from '../../shared/errors'
20+
import { ToolkitError, isUserCancelledError } from '../../shared/errors'
2121
import { showConfirmationMessage } from '../../shared/utilities/messages'
2222
import { RemoteSessionError } from '../../shared/remoteSession'
2323
import {
2424
ConnectFromRemoteWorkspaceMessage,
25-
InstanceTypeError,
2625
InstanceTypeInsufficientMemory,
2726
InstanceTypeInsufficientMemoryMessage,
2827
RemoteAccess,
@@ -241,19 +240,29 @@ export async function openRemoteConnect(
241240
return
242241
}
243242

244-
const spaceName = node.spaceApp.SpaceName!
245-
await tryRefreshNode(node)
243+
try {
244+
const spaceName = node.spaceApp.SpaceName!
245+
await tryRefreshNode(node)
246246

247-
const remoteAccess = node.spaceApp.SpaceSettingsSummary?.RemoteAccess
248-
const nodeStatus = node.getStatus()
247+
const remoteAccess = node.spaceApp.SpaceSettingsSummary?.RemoteAccess
248+
const nodeStatus = node.getStatus()
249249

250-
// Route to appropriate handler based on space state
251-
if (nodeStatus === SpaceStatus.RUNNING && remoteAccess !== RemoteAccess.ENABLED) {
252-
return handleRunningSpaceWithDisabledAccess(node, ctx, spaceName, sageMakerClient)
253-
} else if (nodeStatus === SpaceStatus.STOPPED) {
254-
return handleStoppedSpace(node, ctx, spaceName, sageMakerClient)
255-
} else if (nodeStatus === SpaceStatus.RUNNING) {
256-
return handleRunningSpaceWithEnabledAccess(node, ctx, spaceName)
250+
// Route to appropriate handler based on space state
251+
if (nodeStatus === SpaceStatus.RUNNING && remoteAccess !== RemoteAccess.ENABLED) {
252+
return await handleRunningSpaceWithDisabledAccess(node, ctx, spaceName, sageMakerClient)
253+
} else if (nodeStatus === SpaceStatus.STOPPED) {
254+
return await handleStoppedSpace(node, ctx, spaceName, sageMakerClient)
255+
} else if (nodeStatus === SpaceStatus.RUNNING) {
256+
return await handleRunningSpaceWithEnabledAccess(node, ctx, spaceName)
257+
}
258+
} catch (err: any) {
259+
// Suppress errors that don't need additional error messages:
260+
// - User cancellations (checked by isUserCancelledError)
261+
// - SSH config errors (already shown via modal in prepareDevEnvConnection)
262+
if (isUserCancelledError(err) || (err instanceof ToolkitError && err.code === 'SshConfigError')) {
263+
return
264+
}
265+
throw err
257266
}
258267
}
259268

@@ -377,8 +386,10 @@ async function handleRunningSpaceWithDisabledAccess(
377386
)
378387
await tryRemoteConnection(node, ctx, progress)
379388
} catch (err: any) {
380-
// Handle user declining instance type upgrade
381-
if (err.code === InstanceTypeError) {
389+
// Suppress errors that don't need additional error messages:
390+
// - User cancellations (checked by isUserCancelledError)
391+
// - SSH config errors (already shown via modal in prepareDevEnvConnection)
392+
if (isUserCancelledError(err) || (err instanceof ToolkitError && err.code === 'SshConfigError')) {
382393
return
383394
}
384395
throw new ToolkitError(`Remote connection failed: ${err.message}`, {
@@ -424,8 +435,10 @@ async function handleStoppedSpace(
424435
}
425436
)
426437
} catch (err: any) {
427-
// Handle user declining instance type upgrade
428-
if (err.code === InstanceTypeError) {
438+
// Suppress errors that don't need additional error messages:
439+
// - User cancellations (checked by isUserCancelledError)
440+
// - SSH config errors (already shown via modal in prepareDevEnvConnection)
441+
if (isUserCancelledError(err) || (err instanceof ToolkitError && err.code === 'SshConfigError')) {
429442
return
430443
}
431444
throw new ToolkitError(`Remote connection failed: ${(err as Error).message}`, {

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ export const ConnectFromRemoteWorkspaceMessage =
77
'Unable to establish new remote connection. Your last active VS Code window is connected to a remote workspace. To open a new SageMaker Studio connection, select your local VS Code window and try again.'
88

99
export const InstanceTypeError = 'InstanceTypeError'
10+
export const SshConfigError = 'SshConfigError'
1011

1112
export const InstanceTypeMinimum = 'ml.t3.large'
1213

@@ -46,6 +47,10 @@ export const InstanceTypeNotSelectedMessage = (spaceName: string) => {
4647
export const RemoteAccessRequiredMessage =
4748
'This space requires remote access to be enabled.\nWould you like to restart the space and connect?\nAny unsaved work will be lost.'
4849

50+
export const SshConfigErrorMessage = () => {
51+
return `Unable to connect. Your SSH config file contains errors. Fix the errors to continue.`
52+
}
53+
4954
export const SmusDeeplinkSessionExpiredError = {
5055
title: 'Session Disconnected',
5156
message:

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

Lines changed: 24 additions & 1 deletion
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 { Result } from '../../shared/utilities/result'
@@ -23,6 +23,7 @@ import { ToolkitError } from '../../shared/errors'
2323
import { SagemakerSpaceNode } from './explorer/sagemakerSpaceNode'
2424
import { sleep } from '../../shared/utilities/timeoutUtils'
2525
import { SagemakerUnifiedStudioSpaceNode } from '../../sagemakerunifiedstudio/explorer/nodes/sageMakerUnifiedStudioSpaceNode'
26+
import { SshConfigError, SshConfigErrorMessage } from './constants'
2627
import globals from '../../shared/extensionGlobals'
2728

2829
const logger = getLogger('sagemaker')
@@ -162,6 +163,28 @@ export async function prepareDevEnvConnection(
162163
const config = await sshConfig.ensureValid()
163164
if (config.isErr()) {
164165
const err = config.err()
166+
logger.error(`sagemaker: failed to add ssh config section: ${err.message}`)
167+
168+
if (err instanceof ToolkitError && err.code === 'SshCheckFailed') {
169+
const sshConfigPath = getSshConfigPath()
170+
const openConfigButton = 'Open SSH Config'
171+
const resp = await vscode.window.showErrorMessage(
172+
SshConfigErrorMessage(),
173+
{ modal: true, detail: err.message },
174+
openConfigButton
175+
)
176+
177+
if (resp === openConfigButton) {
178+
void vscode.window.showTextDocument(vscode.Uri.file(sshConfigPath))
179+
}
180+
181+
// Throw error to stop the connection flow
182+
// User is already notified via modal above, downstream handlers check the error code
183+
throw new ToolkitError('Unable to connect: SSH configuration contains errors', {
184+
code: SshConfigError,
185+
})
186+
}
187+
165188
const logPrefix = connectionType === 'sm_hp' ? 'hyperpod' : 'sagemaker'
166189
logger.error(`${logPrefix}: failed to add ssh config section: ${err.message}`)
167190
throw err

packages/core/src/shared/sshConfig.ts

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,20 @@ export class SshConfig {
8585
protected async matchSshSection() {
8686
const result = await this.checkSshOnHost()
8787
if (result.exitCode !== 0) {
88-
return Result.err(result.error ?? new Error(`ssh check against host failed: ${result.exitCode}`))
88+
// Format stderr error message for display to user
89+
let errorMessage = result.stderr?.trim() || `ssh check against host failed: ${result.exitCode}`
90+
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"
93+
errorMessage = errorMessage.replace(new RegExp(`${sshConfigPath}:? `, 'g'), '').trim()
94+
95+
if (result.error) {
96+
// System level error
97+
return Result.err(ToolkitError.chain(result.error, errorMessage))
98+
}
99+
100+
// SSH ran but returned error exit code
101+
return Result.err(new ToolkitError(errorMessage, { code: 'SshCheckFailed' }))
89102
}
90103
const matches = result.stdout.match(this.proxyCommandRegExp)
91104
return Result.ok(matches?.[0])

packages/core/src/test/shared/sshConfig.test.ts

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ class MockSshConfig extends SshConfig {
2626
// State variables to track logic flow.
2727
public testIsWin: boolean = false
2828
public configSection: string = ''
29+
public exitCodeOverride: number = 0
2930

3031
public async getProxyCommandWrapper(command: string): Promise<Result<string, ToolkitError>> {
3132
return await this.getProxyCommand(command)
@@ -51,7 +52,7 @@ class MockSshConfig extends SshConfig {
5152

5253
protected override async checkSshOnHost(): Promise<ChildProcessResult> {
5354
return {
54-
exitCode: 0,
55+
exitCode: this.exitCodeOverride,
5556
error: undefined,
5657
stdout: this.configSection,
5758
stderr: '',
@@ -95,6 +96,10 @@ describe('VscodeRemoteSshConfig', async function () {
9596
})
9697

9798
describe('matchSshSection', async function () {
99+
beforeEach(function () {
100+
config.exitCodeOverride = 0
101+
})
102+
98103
it('returns ok with match when proxycommand is present', async function () {
99104
const testSection = `proxycommandfdsafdsafd${testProxyCommand}sa342432`
100105
const result = await config.testMatchSshSection(testSection)
@@ -110,6 +115,16 @@ describe('VscodeRemoteSshConfig', async function () {
110115
const match = result.unwrap()
111116
assert.strictEqual(match, undefined)
112117
})
118+
119+
it('returns error when ssh check fails with non-zero exit code', async function () {
120+
config.exitCodeOverride = 255
121+
const testSection = `some config`
122+
const result = await config.testMatchSshSection(testSection)
123+
assert.ok(result.isErr())
124+
const error = result.err()
125+
assert.ok(error.message.includes('ssh check against host failed'))
126+
assert.ok(error.message.includes('255'))
127+
})
113128
})
114129

115130
describe('verifySSHHost', async function () {
@@ -122,6 +137,7 @@ describe('VscodeRemoteSshConfig', async function () {
122137
})
123138

124139
beforeEach(function () {
140+
config.exitCodeOverride = 0
125141
promptUserToConfigureSshConfigStub.resetHistory()
126142
})
127143

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)