Skip to content

Commit b40b9bd

Browse files
bharathGuntamaduguguntamblaileni-aws
authored
fix(smus): Improve error handling and handle No projects and spaces scenarios (aws#2200)
## Problem SageMaker Unified Studio explorer failed to handle scenarios when no projects or spaces are available, causing poor user experience and potential errors. ## Solution - Enhanced DataZone client error handling for missing resources - Added proper handling for empty project and space lists in explorer nodes - Improved error messages and fallback behavior in SageMaker space integration - Updated utility functions to gracefully handle null/undefined scenarios - Should fix the Space tooltip not to include the userId and SM domainId, and fixes the the scenario of after sign out with project already selected - If we sign in again with different domain, project from other domain is still shows up. Thanks to Bhargava for the fix. - Added/Updated the test cases accordingly --- - 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. Co-authored-by: guntamb <[email protected]> Co-authored-by: Laxman Reddy <[email protected]>
1 parent 0ffb792 commit b40b9bd

13 files changed

+788
-176
lines changed

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -131,8 +131,10 @@ export class SagemakerSpace {
131131
const domainId = this.spaceApp?.DomainId ?? '-'
132132
const owner = this.spaceApp?.OwnershipSettingsSummary?.OwnerUserProfileName || '-'
133133
const instanceType = this.spaceApp?.App?.ResourceSpec?.InstanceType ?? '-'
134-
135-
return `**Space:** ${spaceName} \n\n**Application:** ${appType} \n\n**Domain ID:** ${domainId} \n\n**User Profile:** ${owner} \n\n**Instance:** ${instanceType}`
134+
if (this.isSMUSSpace) {
135+
return `**Space:** ${spaceName} \n\n**Application:** ${appType} \n\n**Instance Type:** ${instanceType}`
136+
}
137+
return `**Space:** ${spaceName} \n\n**Application:** ${appType} \n\n**Domain ID:** ${domainId} \n\n**User Profile:** ${owner}`
136138
}
137139

138140
public getAppIcon() {

packages/core/src/sagemakerunifiedstudio/explorer/nodes/sageMakerUnifiedStudioConnectionParentNode.ts

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,16 @@ export class SageMakerUnifiedStudioConnectionParentNode implements TreeNode {
3838
this.connectionType
3939
)
4040
const childrenNodes = []
41-
if (!this.connections?.items) {
42-
return []
41+
if (!this.connections?.items || this.connections.items.length === 0) {
42+
return [
43+
{
44+
id: 'smusNoConnections',
45+
resource: {},
46+
getTreeItem: () =>
47+
new vscode.TreeItem('[No connections found]', vscode.TreeItemCollapsibleState.None),
48+
getParent: () => this,
49+
},
50+
]
4351
}
4452
for (const connection of this.connections.items) {
4553
childrenNodes.push(new SageMakerUnifiedStudioConnectionNode(this, connection))

packages/core/src/sagemakerunifiedstudio/explorer/nodes/sageMakerUnifiedStudioProjectNode.ts

Lines changed: 60 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,13 @@ import { getLogger } from '../../../shared/logger/logger'
99
import { telemetry } from '../../../shared/telemetry/telemetry'
1010
import { AwsCredentialIdentity } from '@aws-sdk/types'
1111
import { SageMakerUnifiedStudioDataNode } from './sageMakerUnifiedStudioDataNode'
12-
import { DataZoneProject } from '../../shared/client/datazoneClient'
12+
import { DataZoneClient, DataZoneProject } from '../../shared/client/datazoneClient'
1313
import { SageMakerUnifiedStudioRootNode } from './sageMakerUnifiedStudioRootNode'
1414
import { SagemakerClient } from '../../../shared/clients/sagemaker'
1515
import { SmusAuthenticationProvider } from '../../auth/providers/smusAuthenticationProvider'
1616
import { SageMakerUnifiedStudioComputeNode } from './sageMakerUnifiedStudioComputeNode'
17+
import { getIcon } from '../../../shared/icons'
18+
import { SmusUtils } from '../../shared/smusUtils'
1719

1820
/**
1921
* Tree node representing a SageMaker Unified Studio project
@@ -36,18 +38,25 @@ export class SageMakerUnifiedStudioProjectNode implements TreeNode {
3638

3739
public async getTreeItem(): Promise<vscode.TreeItem> {
3840
if (this.project) {
39-
const item = new vscode.TreeItem('Project: ' + this.project.name, vscode.TreeItemCollapsibleState.Collapsed)
41+
const item = new vscode.TreeItem('Project: ' + this.project.name, vscode.TreeItemCollapsibleState.Expanded)
4042
item.contextValue = 'smusSelectedProject'
4143
item.tooltip = `Project: ${this.project.name}\nID: ${this.project.id}`
44+
item.iconPath = getIcon('vscode-folder-opened')
4245
return item
4346
}
47+
4448
const item = new vscode.TreeItem('Select a project', vscode.TreeItemCollapsibleState.None)
4549
item.contextValue = 'smusProjectSelectPicker'
4650
item.command = {
4751
command: 'aws.smus.projectView',
4852
title: 'Select Project',
4953
arguments: [this],
5054
}
55+
item.iconPath = getIcon('vscode-folder-opened')
56+
57+
// Auto-invoke project selection after sign-in
58+
void vscode.commands.executeCommand('aws.smus.projectView', this)
59+
5160
return item
5261
}
5362

@@ -61,6 +70,24 @@ export class SageMakerUnifiedStudioProjectNode implements TreeNode {
6170
result: 'Succeeded',
6271
passive: false,
6372
})
73+
const hasAccess = await this.checkProjectAccess(this.project.id)
74+
if (!hasAccess) {
75+
return [
76+
{
77+
id: 'smusProjectAccessDenied',
78+
resource: {},
79+
getTreeItem: () => {
80+
const item = new vscode.TreeItem(
81+
'You are not a member of this project. Contact any of its owners to add you as a member.',
82+
vscode.TreeItemCollapsibleState.None
83+
)
84+
return item
85+
},
86+
getParent: () => this,
87+
},
88+
]
89+
}
90+
6491
const dataNode = new SageMakerUnifiedStudioDataNode(this)
6592
this.sagemakerClient = await this.initializeSagemakerClient(
6693
this.authProvider.activeConnection?.ssoRegion || 'us-east-1'
@@ -87,17 +114,44 @@ export class SageMakerUnifiedStudioProjectNode implements TreeNode {
87114
}
88115

89116
public async setProject(project: any): Promise<void> {
117+
await this.cleanupProjectResources()
118+
this.project = project
119+
}
120+
121+
public getProject(): DataZoneProject | undefined {
122+
return this.project
123+
}
124+
125+
public async clearProject(): Promise<void> {
126+
await this.cleanupProjectResources()
127+
this.project = undefined
128+
await this.refreshNode()
129+
}
130+
131+
private async cleanupProjectResources(): Promise<void> {
90132
await this.authProvider.invalidateAllProjectCredentialsInCache()
91133
if (this.sagemakerClient) {
92134
this.sagemakerClient.dispose()
93135
this.sagemakerClient = undefined
94136
}
95-
this.project = project
96-
await this.refreshNode()
97137
}
98138

99-
public getProject(): DataZoneProject | undefined {
100-
return this.project
139+
private async checkProjectAccess(projectId: string): Promise<boolean> {
140+
try {
141+
const dzClient = await DataZoneClient.getInstance(this.authProvider)
142+
const userId = await dzClient.getUserId()
143+
if (!userId) {
144+
return false
145+
}
146+
const ssoUserProfileId = SmusUtils.extractSSOIdFromUserId(userId)
147+
const memberships = await dzClient.fetchAllProjectMemberships(projectId)
148+
const hasAccess = memberships.some((member) => member.memberDetails?.user?.userId === ssoUserProfileId)
149+
this.logger.debug(`Project access check for user ${ssoUserProfileId}: ${hasAccess}`)
150+
return hasAccess
151+
} catch (err) {
152+
this.logger.error('Failed to check project access: %s', (err as Error).message)
153+
return false
154+
}
101155
}
102156

103157
private async initializeSagemakerClient(regionCode: string): Promise<SagemakerClient> {

packages/core/src/sagemakerunifiedstudio/explorer/nodes/sageMakerUnifiedStudioRootNode.ts

Lines changed: 37 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@ import { SmusAuthenticationProvider } from '../../auth/providers/smusAuthenticat
1919
const contextValueSmusRoot = 'sageMakerUnifiedStudioRoot'
2020
const contextValueSmusLogin = 'sageMakerUnifiedStudioLogin'
2121
const contextValueSmusLearnMore = 'sageMakerUnifiedStudioLearnMore'
22+
const projectPickerTitle = 'Select a SageMaker Unified Studio project you want to open'
23+
const projectPickerPlaceholder = 'Select project'
2224

2325
export class SageMakerUnifiedStudioRootNode implements TreeNode {
2426
public readonly id = 'smusRootNode'
@@ -38,7 +40,9 @@ export class SageMakerUnifiedStudioRootNode implements TreeNode {
3840
this.projectNode = new SageMakerUnifiedStudioProjectNode(this, this.authProvider, this.extensionContext)
3941

4042
// Subscribe to auth provider connection changes to refresh the node
41-
this.authProvider.onDidChange(() => {
43+
this.authProvider.onDidChange(async () => {
44+
// Clear the project when connection changes
45+
await this.projectNode.clearProject()
4246
this.onDidChangeEmitter.fire()
4347
})
4448
}
@@ -352,11 +356,7 @@ export async function selectSMUSProject(projectNode?: SageMakerUnifiedStudioProj
352356

353357
const smusProjects = allProjects
354358

355-
if (smusProjects.length === 0) {
356-
void vscode.window.showInformationMessage('No projects found in the domain')
357-
return
358-
}
359-
// Process projects: sort by updatedAt, filter out current project, and map to quick pick items
359+
// Process projects: sort by updatedAt, and map to quick pick items
360360
const items = [...smusProjects]
361361
.sort(
362362
(a, b) =>
@@ -366,28 +366,45 @@ export async function selectSMUSProject(projectNode?: SageMakerUnifiedStudioProj
366366
.filter(
367367
(project) =>
368368
// Filter out the Generative AI Model Governance project that is part of QiuckStart resources
369-
project.name !== 'GenerativeAIModelGovernanceProject' &&
370-
(!projectNode?.getProject() || project.id !== projectNode.getProject()?.id)
369+
project.name !== 'GenerativeAIModelGovernanceProject'
371370
)
372371
.map((project) => ({
373372
label: project.name,
374373
detail: 'ID: ' + project.id,
375374
description: project.description,
376375
data: project,
377376
}))
378-
379-
const quickPick = createQuickPick(items, {
380-
title: 'Select a SageMaker Unified Studio project you want to open',
381-
placeholder: 'Select project',
382-
})
383-
384-
const selectedProject = await quickPick.prompt()
385-
if (selectedProject && projectNode) {
386-
await projectNode.setProject(selectedProject)
387-
// Refresh the entire tree view
388-
await vscode.commands.executeCommand('aws.smus.rootView.refresh')
377+
if (items.length === 0) {
378+
logger.info('No projects found in the domain')
379+
void vscode.window.showInformationMessage('No projects found in the domain')
380+
// If no projects are found, show "No projects found" in the quick pick
381+
const quickPickItem = [
382+
{
383+
label: 'No projects found',
384+
detail: '',
385+
description: '',
386+
data: {},
387+
},
388+
]
389+
const quickPick = createQuickPick(quickPickItem, {
390+
title: projectPickerTitle,
391+
placeholder: projectPickerPlaceholder,
392+
})
393+
await quickPick.prompt()
394+
} else {
395+
const quickPick = createQuickPick(items, {
396+
title: projectPickerTitle,
397+
placeholder: projectPickerPlaceholder,
398+
})
399+
400+
const selectedProject = await quickPick.prompt()
401+
if (selectedProject && !('type' in selectedProject) && projectNode) {
402+
await projectNode.setProject(selectedProject)
403+
// Refresh the entire tree view
404+
await vscode.commands.executeCommand('aws.smus.rootView.refresh')
405+
}
406+
return selectedProject
389407
}
390-
return selectedProject
391408
} catch (err) {
392409
logger.error('Failed to select project: %s', (err as Error).message)
393410
void vscode.window.showErrorMessage(`Failed to select project: ${(err as Error).message}`)

packages/core/src/sagemakerunifiedstudio/explorer/nodes/sageMakerUnifiedStudioSpacesParentNode.ts

Lines changed: 22 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import { UserProfileMetadata } from '../../../awsService/sagemaker/explorer/sage
1717
import { SagemakerUnifiedStudioSpaceNode } from './sageMakerUnifiedStudioSpaceNode'
1818
import { PollingSet } from '../../../shared/utilities/pollingSet'
1919
import { SmusAuthenticationProvider } from '../../auth/providers/smusAuthenticationProvider'
20+
import { SmusUtils } from '../../shared/smusUtils'
2021

2122
export class SageMakerUnifiedStudioSpacesParentNode implements TreeNode {
2223
public readonly id = 'smusSpacesParentNode'
@@ -55,11 +56,29 @@ export class SageMakerUnifiedStudioSpacesParentNode implements TreeNode {
5556
}
5657

5758
public async getChildren(): Promise<TreeNode[]> {
58-
await this.updateChildren()
59+
try {
60+
await this.updateChildren()
61+
} catch (err) {
62+
return this.getNoSpacesFoundChildren()
63+
}
5964
const nodes = [...this.sagemakerSpaceNodes.values()]
65+
if (nodes.length === 0) {
66+
return this.getNoSpacesFoundChildren()
67+
}
6068
return nodes
6169
}
6270

71+
private getNoSpacesFoundChildren(): TreeNode[] {
72+
return [
73+
{
74+
id: 'smusNoSpaces',
75+
resource: {},
76+
getTreeItem: () => new vscode.TreeItem('[No Spaces found]', vscode.TreeItemCollapsibleState.None),
77+
getParent: () => this,
78+
},
79+
]
80+
}
81+
6382
public getParent(): TreeNode | undefined {
6483
return this.parent
6584
}
@@ -104,7 +123,7 @@ export class SageMakerUnifiedStudioSpacesParentNode implements TreeNode {
104123
const toolingEnvId = await datazoneClient
105124
.getToolingEnvironmentId(datazoneClient.getDomainId(), this.projectId)
106125
.catch((err) => {
107-
this.logger.error('Failed to get tooling environment ID: %s', err as Error)
126+
this.logger.error('Failed to get tooling environment ID for project %s', this.projectId)
108127
throw new Error(`Failed to get tooling environment ID: ${err.message}`)
109128
})
110129
if (!toolingEnvId) {
@@ -144,7 +163,7 @@ export class SageMakerUnifiedStudioSpacesParentNode implements TreeNode {
144163
const datazoneClient = await DataZoneClient.getInstance(this.authProvider)
145164
// Will be of format: 'ABCA4NU3S7PEOLDQPLXYZ:user-12345678-d061-70a4-0bf2-eeee67a6ab12'
146165
const userId = await datazoneClient.getUserId()
147-
const ssoUserProfileId = this.extractSSOIdFromUserId(userId || '')
166+
const ssoUserProfileId = SmusUtils.extractSSOIdFromUserId(userId || '')
148167
const sagemakerDomainId = await this.getSageMakerDomainId()
149168
const [spaceApps, domains] = await this.sagemakerClient.fetchSpaceAppsAndDomains(
150169
sagemakerDomainId,
@@ -188,13 +207,4 @@ export class SageMakerUnifiedStudioSpacesParentNode implements TreeNode {
188207
)
189208
)
190209
}
191-
192-
private extractSSOIdFromUserId(userId: string): string {
193-
const match = userId.match(/user-(.+)$/)
194-
if (!match) {
195-
this.logger.error(`Invalid UserId format: ${userId}`)
196-
throw new Error(`Invalid UserId format: ${userId}`)
197-
}
198-
return match[1]
199-
}
200210
}

0 commit comments

Comments
 (0)