Skip to content

Commit a942068

Browse files
vpbhargavBhargava Varadharajan
andauthored
fix(smus): Fix potential race condition bug on getting context (aws#8011)
**Description** We were seeing intermittent issues where sometimes the context was being returned as undefined. Setting it at the start of the activation to ensure the the context is available always. **Motivation** Bug fix **Testing Done** Tested manually locally. ## Problem ## Solution --- - 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: Bhargava Varadharajan <[email protected]>
1 parent 24a6f5f commit a942068

File tree

3 files changed

+281
-1
lines changed

3 files changed

+281
-1
lines changed

packages/core/src/sagemakerunifiedstudio/activation.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,15 @@ import { activate as activateConnectionMagicsSelector } from './connectionMagics
88
import { activate as activateExplorer } from './explorer/activation'
99
import { isSageMaker } from '../shared/extensionUtilities'
1010
import { initializeResourceMetadata } from './shared/utils/resourceMetadataUtils'
11+
import { setContext } from '../shared/vscode/setContext'
12+
import { SmusUtils } from './shared/smusUtils'
1113

1214
export async function activate(extensionContext: vscode.ExtensionContext): Promise<void> {
1315
// Only run when environment is a SageMaker Unified Studio space
1416
if (isSageMaker('SMUS') || isSageMaker('SMUS-SPACE-REMOTE-ACCESS')) {
1517
await initializeResourceMetadata()
18+
// Setting context before any getContext calls to avoid potential race conditions.
19+
await setContext('aws.smus.inSmusSpaceEnvironment', SmusUtils.isInSmusSpaceEnvironment())
1620
await activateConnectionMagicsSelector(extensionContext)
1721
}
1822
await activateExplorer(extensionContext)
Lines changed: 273 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,273 @@
1+
/*!
2+
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
3+
* SPDX-License-Identifier: Apache-2.0
4+
*/
5+
6+
import assert from 'assert'
7+
import sinon from 'sinon'
8+
import * as vscode from 'vscode'
9+
import { activate } from '../../sagemakerunifiedstudio/activation'
10+
import * as extensionUtilities from '../../shared/extensionUtilities'
11+
import * as connectionMagicsSelectorActivation from '../../sagemakerunifiedstudio/connectionMagicsSelector/activation'
12+
import * as explorerActivation from '../../sagemakerunifiedstudio/explorer/activation'
13+
import * as resourceMetadataUtils from '../../sagemakerunifiedstudio/shared/utils/resourceMetadataUtils'
14+
import * as setContext from '../../shared/vscode/setContext'
15+
import { SmusUtils } from '../../sagemakerunifiedstudio/shared/smusUtils'
16+
17+
describe('SageMaker Unified Studio Main Activation', function () {
18+
let mockExtensionContext: vscode.ExtensionContext
19+
let isSageMakerStub: sinon.SinonStub
20+
let initializeResourceMetadataStub: sinon.SinonStub
21+
let setContextStub: sinon.SinonStub
22+
let isInSmusSpaceEnvironmentStub: sinon.SinonStub
23+
let activateConnectionMagicsSelectorStub: sinon.SinonStub
24+
let activateExplorerStub: sinon.SinonStub
25+
26+
beforeEach(function () {
27+
mockExtensionContext = {
28+
subscriptions: [],
29+
extensionPath: '/test/path',
30+
globalState: {
31+
get: sinon.stub(),
32+
update: sinon.stub(),
33+
},
34+
workspaceState: {
35+
get: sinon.stub(),
36+
update: sinon.stub(),
37+
},
38+
} as any
39+
40+
// Stub all dependencies
41+
isSageMakerStub = sinon.stub(extensionUtilities, 'isSageMaker')
42+
initializeResourceMetadataStub = sinon.stub(resourceMetadataUtils, 'initializeResourceMetadata')
43+
setContextStub = sinon.stub(setContext, 'setContext')
44+
isInSmusSpaceEnvironmentStub = sinon.stub(SmusUtils, 'isInSmusSpaceEnvironment')
45+
activateConnectionMagicsSelectorStub = sinon.stub(connectionMagicsSelectorActivation, 'activate')
46+
activateExplorerStub = sinon.stub(explorerActivation, 'activate')
47+
48+
// Set default return values
49+
isSageMakerStub.returns(false)
50+
initializeResourceMetadataStub.resolves()
51+
setContextStub.resolves()
52+
isInSmusSpaceEnvironmentStub.returns(false)
53+
activateConnectionMagicsSelectorStub.resolves()
54+
activateExplorerStub.resolves()
55+
})
56+
57+
afterEach(function () {
58+
sinon.restore()
59+
})
60+
61+
describe('activate function', function () {
62+
it('should always activate explorer regardless of environment', async function () {
63+
isSageMakerStub.returns(false)
64+
65+
await activate(mockExtensionContext)
66+
67+
assert.ok(activateExplorerStub.calledOnceWith(mockExtensionContext))
68+
})
69+
70+
it('should not initialize SMUS components when not in SageMaker environment', async function () {
71+
isSageMakerStub.returns(false)
72+
73+
await activate(mockExtensionContext)
74+
75+
assert.ok(initializeResourceMetadataStub.notCalled)
76+
assert.ok(setContextStub.notCalled)
77+
assert.ok(activateConnectionMagicsSelectorStub.notCalled)
78+
assert.ok(activateExplorerStub.calledOnceWith(mockExtensionContext))
79+
})
80+
81+
it('should initialize SMUS components when in SMUS environment', async function () {
82+
isSageMakerStub.withArgs('SMUS').returns(true)
83+
isSageMakerStub.withArgs('SMUS-SPACE-REMOTE-ACCESS').returns(false)
84+
isInSmusSpaceEnvironmentStub.returns(true)
85+
86+
await activate(mockExtensionContext)
87+
88+
assert.ok(initializeResourceMetadataStub.calledOnce)
89+
assert.ok(setContextStub.calledOnceWith('aws.smus.inSmusSpaceEnvironment', true))
90+
assert.ok(activateConnectionMagicsSelectorStub.calledOnceWith(mockExtensionContext))
91+
assert.ok(activateExplorerStub.calledOnceWith(mockExtensionContext))
92+
})
93+
94+
it('should initialize SMUS components when in SMUS-SPACE-REMOTE-ACCESS environment', async function () {
95+
isSageMakerStub.withArgs('SMUS').returns(false)
96+
isSageMakerStub.withArgs('SMUS-SPACE-REMOTE-ACCESS').returns(true)
97+
isInSmusSpaceEnvironmentStub.returns(false)
98+
99+
await activate(mockExtensionContext)
100+
101+
assert.ok(initializeResourceMetadataStub.calledOnce)
102+
assert.ok(setContextStub.calledOnceWith('aws.smus.inSmusSpaceEnvironment', false))
103+
assert.ok(activateConnectionMagicsSelectorStub.calledOnceWith(mockExtensionContext))
104+
assert.ok(activateExplorerStub.calledOnceWith(mockExtensionContext))
105+
})
106+
107+
it('should call functions in correct order for SMUS environment', async function () {
108+
isSageMakerStub.withArgs('SMUS').returns(true)
109+
isSageMakerStub.withArgs('SMUS-SPACE-REMOTE-ACCESS').returns(false)
110+
isInSmusSpaceEnvironmentStub.returns(true)
111+
112+
await activate(mockExtensionContext)
113+
114+
// Verify the order of calls
115+
assert.ok(initializeResourceMetadataStub.calledBefore(setContextStub))
116+
assert.ok(setContextStub.calledBefore(activateConnectionMagicsSelectorStub))
117+
assert.ok(activateConnectionMagicsSelectorStub.calledBefore(activateExplorerStub))
118+
})
119+
120+
it('should handle initializeResourceMetadata errors', async function () {
121+
isSageMakerStub.withArgs('SMUS').returns(true)
122+
const error = new Error('Resource metadata initialization failed')
123+
initializeResourceMetadataStub.rejects(error)
124+
125+
await assert.rejects(() => activate(mockExtensionContext), /Resource metadata initialization failed/)
126+
127+
assert.ok(initializeResourceMetadataStub.calledOnce)
128+
assert.ok(setContextStub.notCalled)
129+
assert.ok(activateConnectionMagicsSelectorStub.notCalled)
130+
})
131+
132+
it('should handle setContext errors', async function () {
133+
isSageMakerStub.withArgs('SMUS').returns(true)
134+
isInSmusSpaceEnvironmentStub.returns(true)
135+
const error = new Error('Set context failed')
136+
setContextStub.rejects(error)
137+
138+
await assert.rejects(() => activate(mockExtensionContext), /Set context failed/)
139+
140+
assert.ok(initializeResourceMetadataStub.calledOnce)
141+
assert.ok(setContextStub.calledOnce)
142+
assert.ok(activateConnectionMagicsSelectorStub.notCalled)
143+
})
144+
145+
it('should handle connectionMagicsSelector activation errors', async function () {
146+
isSageMakerStub.withArgs('SMUS').returns(true)
147+
isInSmusSpaceEnvironmentStub.returns(true)
148+
const error = new Error('Connection magics selector activation failed')
149+
activateConnectionMagicsSelectorStub.rejects(error)
150+
151+
await assert.rejects(() => activate(mockExtensionContext), /Connection magics selector activation failed/)
152+
153+
assert.ok(initializeResourceMetadataStub.calledOnce)
154+
assert.ok(setContextStub.calledOnce)
155+
assert.ok(activateConnectionMagicsSelectorStub.calledOnce)
156+
})
157+
158+
it('should handle explorer activation errors', async function () {
159+
const error = new Error('Explorer activation failed')
160+
activateExplorerStub.rejects(error)
161+
162+
await assert.rejects(() => activate(mockExtensionContext), /Explorer activation failed/)
163+
164+
assert.ok(activateExplorerStub.calledOnce)
165+
})
166+
167+
it('should pass correct extension context to all activation functions', async function () {
168+
isSageMakerStub.withArgs('SMUS').returns(true)
169+
isInSmusSpaceEnvironmentStub.returns(true)
170+
171+
await activate(mockExtensionContext)
172+
173+
assert.ok(activateConnectionMagicsSelectorStub.calledWith(mockExtensionContext))
174+
assert.ok(activateExplorerStub.calledWith(mockExtensionContext))
175+
})
176+
})
177+
178+
describe('environment detection logic', function () {
179+
it('should check both SMUS and SMUS-SPACE-REMOTE-ACCESS environments', async function () {
180+
isSageMakerStub.withArgs('SMUS').returns(false)
181+
isSageMakerStub.withArgs('SMUS-SPACE-REMOTE-ACCESS').returns(false)
182+
183+
await activate(mockExtensionContext)
184+
185+
assert.ok(isSageMakerStub.calledWith('SMUS'))
186+
assert.ok(isSageMakerStub.calledWith('SMUS-SPACE-REMOTE-ACCESS'))
187+
})
188+
189+
it('should activate SMUS components if either environment check returns true', async function () {
190+
// Test case 1: Only SMUS returns true
191+
isSageMakerStub.withArgs('SMUS').returns(true)
192+
isSageMakerStub.withArgs('SMUS-SPACE-REMOTE-ACCESS').returns(false)
193+
isInSmusSpaceEnvironmentStub.returns(true)
194+
195+
await activate(mockExtensionContext)
196+
197+
assert.ok(initializeResourceMetadataStub.calledOnce)
198+
assert.ok(activateConnectionMagicsSelectorStub.calledOnce)
199+
200+
// Reset stubs for second test
201+
initializeResourceMetadataStub.resetHistory()
202+
activateConnectionMagicsSelectorStub.resetHistory()
203+
204+
// Test case 2: Only SMUS-SPACE-REMOTE-ACCESS returns true
205+
isSageMakerStub.withArgs('SMUS').returns(false)
206+
isSageMakerStub.withArgs('SMUS-SPACE-REMOTE-ACCESS').returns(true)
207+
isInSmusSpaceEnvironmentStub.returns(false)
208+
209+
await activate(mockExtensionContext)
210+
211+
assert.ok(initializeResourceMetadataStub.calledOnce)
212+
assert.ok(activateConnectionMagicsSelectorStub.calledOnce)
213+
})
214+
215+
it('should use SmusUtils.isInSmusSpaceEnvironment() result for context setting', async function () {
216+
isSageMakerStub.withArgs('SMUS').returns(true)
217+
218+
// Test with true
219+
isInSmusSpaceEnvironmentStub.returns(true)
220+
await activate(mockExtensionContext)
221+
assert.ok(setContextStub.calledWith('aws.smus.inSmusSpaceEnvironment', true))
222+
223+
// Reset and test with false
224+
setContextStub.resetHistory()
225+
isInSmusSpaceEnvironmentStub.returns(false)
226+
await activate(mockExtensionContext)
227+
assert.ok(setContextStub.calledWith('aws.smus.inSmusSpaceEnvironment', false))
228+
})
229+
})
230+
231+
describe('integration scenarios', function () {
232+
it('should handle mixed success and failure scenarios gracefully', async function () {
233+
isSageMakerStub.withArgs('SMUS').returns(true)
234+
isInSmusSpaceEnvironmentStub.returns(true)
235+
236+
// initializeResourceMetadata succeeds, setContext fails
237+
const setContextError = new Error('Context setting failed')
238+
setContextStub.rejects(setContextError)
239+
240+
await assert.rejects(() => activate(mockExtensionContext), /Context setting failed/)
241+
242+
// Verify that initializeResourceMetadata was called but subsequent functions were not
243+
assert.ok(initializeResourceMetadataStub.calledOnce)
244+
assert.ok(setContextStub.calledOnce)
245+
assert.ok(activateConnectionMagicsSelectorStub.notCalled)
246+
assert.ok(activateExplorerStub.notCalled)
247+
})
248+
249+
it('should complete successfully when all components initialize properly', async function () {
250+
isSageMakerStub.withArgs('SMUS').returns(true)
251+
isSageMakerStub.withArgs('SMUS-SPACE-REMOTE-ACCESS').returns(false)
252+
isInSmusSpaceEnvironmentStub.returns(true)
253+
254+
// All functions should succeed
255+
await activate(mockExtensionContext)
256+
257+
// Verify all expected functions were called
258+
assert.ok(initializeResourceMetadataStub.calledOnce)
259+
assert.ok(setContextStub.calledOnce)
260+
assert.ok(activateConnectionMagicsSelectorStub.calledOnce)
261+
assert.ok(activateExplorerStub.calledOnce)
262+
})
263+
264+
it('should handle undefined extension context gracefully', async function () {
265+
const undefinedContext = undefined as any
266+
267+
// Should not throw for undefined context, but let the individual activation functions handle it
268+
await activate(undefinedContext)
269+
270+
assert.ok(activateExplorerStub.calledWith(undefinedContext))
271+
})
272+
})
273+
})

packages/core/src/test/sagemakerunifiedstudio/auth/smusAuthenticationProvider.test.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import * as vscode from 'vscode'
99

1010
// Mock the setContext function BEFORE importing modules that use it
1111
const setContextModule = require('../../../shared/vscode/setContext')
12-
const setContextStubGlobal = sinon.stub(setContextModule, 'setContext').resolves()
1312

1413
import { SmusAuthenticationProvider } from '../../../sagemakerunifiedstudio/auth/providers/smusAuthenticationProvider'
1514
import { SmusConnection } from '../../../sagemakerunifiedstudio/auth/model'
@@ -27,6 +26,7 @@ describe('SmusAuthenticationProvider', function () {
2726
let getSsoInstanceInfoStub: sinon.SinonStub
2827
let isInSmusSpaceEnvironmentStub: sinon.SinonStub
2928
let executeCommandStub: sinon.SinonStub
29+
let setContextStubGlobal: sinon.SinonStub
3030
let mockSecondaryAuthState: {
3131
activeConnection: SmusConnection | undefined
3232
hasSavedConnection: boolean
@@ -57,6 +57,9 @@ describe('SmusAuthenticationProvider', function () {
5757
}
5858

5959
beforeEach(function () {
60+
// Create the setContext stub
61+
setContextStubGlobal = sinon.stub(setContextModule, 'setContext').resolves()
62+
6063
mockAuth = {
6164
createConnection: sinon.stub().resolves(mockSmusConnection),
6265
listConnections: sinon.stub().resolves([]),

0 commit comments

Comments
 (0)