Skip to content

Commit 768b58a

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

File tree

5 files changed

+22
-27
lines changed

5 files changed

+22
-27
lines changed

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

Lines changed: 7 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -17,18 +17,16 @@ 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,
2928
RemoteAccessRequiredMessage,
3029
SpaceStatus,
31-
SshConfigError,
3230
} from './constants'
3331
import { SagemakerUnifiedStudioSpaceNode } from '../../sagemakerunifiedstudio/explorer/nodes/sageMakerUnifiedStudioSpaceNode'
3432

@@ -227,8 +225,8 @@ export async function openRemoteConnect(
227225
return await handleRunningSpaceWithEnabledAccess(node, ctx, spaceName)
228226
}
229227
} catch (err: any) {
230-
// Suppress errors that were already shown to user
231-
if (shouldSuppressError(err)) {
228+
// Don't re-throw errors that were already shown to user or represent user cancellations
229+
if (isUserCancelledError(err)) {
232230
return
233231
}
234232
throw err
@@ -355,8 +353,8 @@ 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+
// Don't re-throw errors that were already shown to user or represent user cancellations
357+
if (isUserCancelledError(err)) {
360358
return
361359
}
362360
throw new ToolkitError(`Remote connection failed: ${err.message}`, {
@@ -402,8 +400,8 @@ async function handleStoppedSpace(
402400
}
403401
)
404402
} catch (err: any) {
405-
// Suppress errors that were already shown to user
406-
if (shouldSuppressError(err)) {
403+
// Don't re-throw errors that were already shown to user or represent user cancellations
404+
if (isUserCancelledError(err)) {
407405
return
408406
}
409407
throw new ToolkitError(`Remote connection failed: ${(err as Error).message}`, {
@@ -434,10 +432,3 @@ async function handleRunningSpaceWithEnabledAccess(
434432
}
435433
)
436434
}
437-
438-
/**
439-
* Helper function to determine if an error should be suppressed (already shown to user)
440-
*/
441-
function shouldSuppressError(err: any): boolean {
442-
return err.code === SshConfigError || err.code === InstanceTypeError
443-
}

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. Your SSH config file contains errors. 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: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -85,10 +85,12 @@ 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-
errorMessage = errorMessage.replace(new RegExp(`${sshConfigPath}:\\s*`, 'g'), '').trim()
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()
9294

9395
if (result.error) {
9496
// System level 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)