Skip to content

Commit 9067548

Browse files
authored
fix(codecatalyst): simplify InactivityMessage aws#5260
Problem: - The "progress" message tests are flaky because of the complex logic, and sometimes fail: ``` 1) InactivityMessages shows expected messages 5 minutes before shutdown on a 60 minute inactivity timeout: AssertionError [ERR_ASSERTION]: Expected 3 messages, but got 2 at assertMessagesShown (src/testInteg/codecatalyst/devEnv.test.ts:182:20) at async Context.<anonymous> (src/testInteg/codecatalyst/devEnv.test.ts:85:9) ``` - Toolkit has complex logic for showing a "progress" message when the codecatalyst dev env timeout is approaching, followed by different logic for showing the "final" message. This isn't worth the complexity because if the user sees the message, clicking it is zero-cost and will happen immediately, otherwise the user isn't active and won't see any of the messages anyway. - The non-modal progress message may be *hidden* if the user enabled vscode's "do not disturb" feature. Solution: - Remove the non-modal "progress" message. Only show the final, modal message **starting 5 min before shutdown**. Display it until user clicks it. Followup to https://taskei.amazon.dev/tasks/IDE-13892
1 parent 9331045 commit 9067548

File tree

3 files changed

+66
-160
lines changed

3 files changed

+66
-160
lines changed

packages/core/src/codecatalyst/devEnv.ts

Lines changed: 14 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,7 @@ import { DevEnvironment } from '../shared/clients/codecatalystClient'
77
import { DevEnvActivity, DevEnvClient } from '../shared/clients/devenvClient'
88
import globals from '../shared/extensionGlobals'
99
import * as vscode from 'vscode'
10-
import { Timeout, waitUntil } from '../shared/utilities/timeoutUtils'
11-
import { showMessageWithCancel } from '../shared/utilities/messages'
12-
import { isCloud9 } from '../shared/extensionUtilities'
10+
import { waitUntil } from '../shared/utilities/timeoutUtils'
1311
import { getLogger } from '../shared/logger'
1412
import { CodeCatalystAuthenticationProvider } from './auth'
1513
import { getThisDevEnv } from './model'
@@ -152,7 +150,6 @@ export function shouldSendActivity(inactivityTimeoutMin: DevEnvironment['inactiv
152150
/** Shows a "Dev env will shutdown in x minutes due to inactivity" warning. */
153151
export class InactivityMessage implements vscode.Disposable {
154152
#showMessageTimer: NodeJS.Timeout | undefined
155-
private progressTimeout = new Timeout(1)
156153
/** Show a message this many minutes before auto-shutdown. */
157154
public readonly shutdownWarningThreshold = 5
158155

@@ -200,12 +197,10 @@ export class InactivityMessage implements vscode.Disposable {
200197

201198
this.clear()
202199
this.show(
203-
devEnvActivity,
204200
Math.round(Math.max(0, minutesSinceActivity + minutesUntilMessage)),
205201
Math.round(Math.max(0, minutesUntilShutdown - minutesUntilMessage)),
206202
userIsActive,
207-
willRefreshOnStaleTimestamp,
208-
oneMin
203+
willRefreshOnStaleTimestamp
209204
).catch(e => {
210205
getLogger().error('InactivityMessage.show failed: %s', (e as Error).message)
211206
})
@@ -246,84 +241,39 @@ export class InactivityMessage implements vscode.Disposable {
246241
* @param willRefreshOnStaleTimestamp sanity checks with the dev env api that the latest activity timestamp
247242
* is the same as what this client has locally. If stale, the warning message
248243
* will be refreshed asynchronously. Returns true if the message will be refreshed.
249-
* @param oneMin Milliseconds in a "minute". Used in tests.
250244
*/
251245
async show(
252-
devEnvActivity: DevEnvActivity,
253246
minutesUserWasInactive: number,
254247
minutesUntilShutdown: number,
255248
userIsActive: () => void,
256-
willRefreshOnStaleTimestamp: () => Promise<boolean>,
257-
oneMin: number
249+
willRefreshOnStaleTimestamp: () => Promise<boolean>
258250
) {
259251
getLogger().debug(
260252
'InactivityMessage.show: minutesUserWasInactive=%d minutesUntilShutdown=%d',
261253
minutesUserWasInactive,
262254
minutesUntilShutdown
263255
)
264256

265-
// Hide any current message.
266-
this.progressTimeout.dispose()
267-
268257
if (await willRefreshOnStaleTimestamp()) {
269258
return
270259
}
271260

272-
if (minutesUntilShutdown <= 1) {
273-
const imHere = "I'm here!"
274-
return vscode.window
275-
.showWarningMessage(
276-
`Your CodeCatalyst Dev Environment has been inactive for ${minutesUserWasInactive} minutes, and will stop soon.`,
277-
{ modal: true },
278-
imHere
279-
)
280-
.then(res => {
281-
if (res === imHere) {
282-
userIsActive()
283-
}
284-
})
285-
}
286-
287-
// Show a new message every minute
288-
this.progressTimeout = new Timeout(oneMin)
289-
this.progressTimeout.token.onCancellationRequested(c => {
290-
getLogger().debug('InactivityMessage.show: CancelEvent.agent=%s', c.agent)
291-
if (c.agent === 'user') {
292-
// User clicked the 'Cancel' button, indicate they are active.
293-
userIsActive()
294-
} else {
295-
// The message timed out, show the updated message.
296-
void this.show(
297-
devEnvActivity,
298-
minutesUserWasInactive + 1,
299-
minutesUntilShutdown - 1,
300-
userIsActive,
301-
willRefreshOnStaleTimestamp,
302-
oneMin
303-
)
304-
}
305-
})
306-
if (isCloud9()) {
307-
// C9 does not support message with progress, so just show a warning message.
308-
return vscode.window
309-
.showWarningMessage(this.getMessage(minutesUserWasInactive, minutesUntilShutdown), 'Cancel')
310-
.then(() => {
311-
this.progressTimeout.cancel()
312-
})
313-
} else {
314-
// Show cancellable "progress" message.
315-
return void showMessageWithCancel(
316-
this.getMessage(minutesUserWasInactive, minutesUntilShutdown),
317-
this.progressTimeout
261+
const imHere = "I'm here!"
262+
return vscode.window
263+
.showWarningMessage(
264+
`Your CodeCatalyst Dev Environment has been inactive for ${minutesUserWasInactive} minutes, and will stop soon.`,
265+
{ modal: true },
266+
imHere
318267
)
319-
}
268+
.then(res => {
269+
if (res === imHere) {
270+
userIsActive()
271+
}
272+
})
320273
}
321274

322275
/** Cancels or disposes the existing message and timer. */
323276
clear() {
324-
// This also dismisses the message if it is currently displaying.
325-
this.progressTimeout.cancel()
326-
327277
if (this.#showMessageTimer) {
328278
clearTimeout(this.#showMessageTimer)
329279
this.#showMessageTimer = undefined
@@ -333,8 +283,4 @@ export class InactivityMessage implements vscode.Disposable {
333283
dispose() {
334284
this.clear()
335285
}
336-
337-
private getMessage(inactiveMinutes: number, remainingMinutes: number) {
338-
return `Your CodeCatalyst Dev Environment has been inactive for ${inactiveMinutes} minutes, and will stop in ${remainingMinutes} minutes.`
339-
}
340286
}

packages/core/src/testInteg/codecatalyst/devEnv.test.ts

Lines changed: 48 additions & 92 deletions
Original file line numberDiff line numberDiff line change
@@ -22,27 +22,31 @@ describe('shouldSendActivity', function () {
2222
})
2323
})
2424

25-
describe('InactivityMessages', function () {
25+
describe('InactivityMessage', function () {
2626
/** Actual minute in prod is this value instead so testing is faster. */
2727
let relativeMinuteMillis: number
2828
let testWindow: TestWindow
2929
let devEnvActivity: sinon.SinonStubbedInstance<DevEnvActivity>
3030
let actualMessages: { message: string; minute: number }[] = []
3131
let inactivityMsg: InactivityMessage
32+
let userActivity: number
3233

3334
beforeEach(function () {
34-
relativeMinuteMillis = 200
35+
relativeMinuteMillis = 100
3536
testWindow = getTestWindow()
3637
inactivityMsg = new InactivityMessage()
38+
userActivity = 0
39+
setInitialOffset(0)
3740

3841
devEnvActivity = sinon.createStubInstance(DevEnvActivity)
39-
// Setup for DevEnvClient stub to call the onUserActivity event callback code when sendActivityUpdate() is called
42+
devEnvActivity.sendActivityUpdate.callsFake(async () => {
43+
userActivity += 1
44+
const timestamp = getLatestTimestamp()
45+
return timestamp
46+
})
4047
devEnvActivity.onActivityUpdate.callsFake(activityCallback => {
41-
devEnvActivity.sendActivityUpdate.callsFake(async () => {
42-
const timestamp = getLatestTimestamp()
43-
activityCallback(timestamp)
44-
return timestamp
45-
})
48+
const timestamp = getLatestTimestamp()
49+
activityCallback(timestamp)
4650
})
4751
devEnvActivity.isLocalActivityStale.callsFake(async () => {
4852
return false
@@ -56,97 +60,47 @@ describe('InactivityMessages', function () {
5660
testWindow.dispose()
5761
})
5862

59-
it('shows expected messages 5 minutes before shutdown on a 15 minute inactivity timeout', async function () {
63+
it('shows warning 5 minutes before shutdown for 15 minute timeout', async function () {
64+
setInitialOffset(9)
6065
await inactivityMsg.init(15, devEnvActivity as unknown as DevEnvActivity, relativeMinuteMillis)
61-
await devEnvActivity.sendActivityUpdate()
62-
await devEnvActivity.sendActivityUpdate()
63-
await devEnvActivity.sendActivityUpdate()
64-
await devEnvActivity.sendActivityUpdate()
65-
await devEnvActivity.sendActivityUpdate()
6666

6767
await assertMessagesShown([
68-
['Your CodeCatalyst Dev Environment has been inactive for 10 minutes, and will stop in 5 minutes.', 10],
69-
['Your CodeCatalyst Dev Environment has been inactive for 11 minutes, and will stop in 4 minutes.', 11],
70-
['Your CodeCatalyst Dev Environment has been inactive for 12 minutes, and will stop in 3 minutes.', 12],
71-
['Your CodeCatalyst Dev Environment has been inactive for 13 minutes, and will stop in 2 minutes.', 13],
72-
['Your CodeCatalyst Dev Environment has been inactive for 14 minutes, and will stop soon.', 14],
68+
['Your CodeCatalyst Dev Environment has been inactive for 10 minutes, and will stop soon.', 10],
7369
])
7470
})
7571

76-
it('shows expected messages 5 minutes before shutdown on a 60 minute inactivity timeout', async function () {
72+
it('shows warning 5 minutes before shutdown for 60 minute timeout', async function () {
73+
setInitialOffset(54)
7774
await inactivityMsg.init(60, devEnvActivity as unknown as DevEnvActivity, relativeMinuteMillis)
78-
setInitialOffset(57)
79-
await devEnvActivity.sendActivityUpdate()
80-
await waitUntil(async () => actualMessages.length > 0, { interval: 10 })
81-
setInitialOffset(58)
82-
await devEnvActivity.sendActivityUpdate()
83-
await waitUntil(async () => actualMessages.length > 2, { interval: 10 })
8475

8576
await assertMessagesShown([
86-
['Your CodeCatalyst Dev Environment has been inactive for 57 minutes, and will stop in 3 minutes.', 0],
87-
['Your CodeCatalyst Dev Environment has been inactive for 58 minutes, and will stop in 2 minutes.', 0],
88-
['Your CodeCatalyst Dev Environment has been inactive for 59 minutes, and will stop soon.', 1],
77+
['Your CodeCatalyst Dev Environment has been inactive for 55 minutes, and will stop soon.', 55],
8978
])
9079
})
9180

92-
it('resets the inactivity countdown when a user clicks on a button in any activity message', async function () {
93-
let isFirstMessage = true
94-
testWindow.onDidShowMessage(async message => {
95-
if (message.message.endsWith('stop soon.')) {
96-
// User hits the "I'm here!" button on the inactivity shutdown message
97-
message.selectItem("I'm here!")
98-
return
99-
}
100-
101-
if (!isFirstMessage) {
102-
return
103-
}
104-
isFirstMessage = false
105-
// User hits the 'Cancel' button on the first inactivity warning message
106-
message.selectItem('Cancel')
107-
})
108-
109-
await inactivityMsg.init(7, devEnvActivity as unknown as DevEnvActivity, relativeMinuteMillis)
110-
await devEnvActivity.sendActivityUpdate()
111-
await devEnvActivity.sendActivityUpdate()
112-
await devEnvActivity.sendActivityUpdate()
113-
await devEnvActivity.sendActivityUpdate()
114-
await devEnvActivity.sendActivityUpdate()
115-
await devEnvActivity.sendActivityUpdate()
116-
await devEnvActivity.sendActivityUpdate()
81+
it('resets inactivity countdown when a user confirms the message', async function () {
82+
await inactivityMsg.init(10, devEnvActivity as unknown as DevEnvActivity, relativeMinuteMillis)
83+
const msg = await testWindow.waitForMessage(/Dev Environment has been inactive/)
84+
assert.deepStrictEqual(userActivity, 1)
85+
msg.selectItem("I'm here!")
86+
await waitUntil(async () => userActivity > 1, { truthy: true, interval: 100, timeout: 5_000 })
87+
assert.deepStrictEqual(userActivity, 2, 'confirming the message should trigger user activity')
11788

11889
await assertMessagesShown([
119-
['Your CodeCatalyst Dev Environment has been inactive for 2 minutes, and will stop in 5 minutes.', 2],
120-
// User clicked 'Cancel' on the warning message so timer was reset
121-
['Your CodeCatalyst Dev Environment has been inactive for 2 minutes, and will stop in 5 minutes.', 4],
122-
['Your CodeCatalyst Dev Environment has been inactive for 3 minutes, and will stop in 4 minutes.', 5],
123-
['Your CodeCatalyst Dev Environment has been inactive for 4 minutes, and will stop in 3 minutes.', 6],
124-
['Your CodeCatalyst Dev Environment has been inactive for 5 minutes, and will stop in 2 minutes.', 7],
125-
['Your CodeCatalyst Dev Environment has been inactive for 6 minutes, and will stop soon.', 8],
126-
// User clicked "I'm here!" on the shutdown message so timer was reset
127-
['Your CodeCatalyst Dev Environment has been inactive for 2 minutes, and will stop in 5 minutes.', 10],
90+
['Your CodeCatalyst Dev Environment has been inactive for 5 minutes, and will stop soon.', 5],
12891
])
12992
})
13093

131-
it('takes in to consideration 2 1/2 minutes have already passed for an inactive external client.', async function () {
94+
it('offsets when 2.5 minutes have already passed for an inactive external client', async function () {
13295
setInitialOffset(2.5)
13396
await inactivityMsg.init(9, devEnvActivity as unknown as DevEnvActivity, relativeMinuteMillis)
134-
await devEnvActivity.sendActivityUpdate()
135-
await devEnvActivity.sendActivityUpdate()
136-
await devEnvActivity.sendActivityUpdate()
137-
await devEnvActivity.sendActivityUpdate()
138-
await devEnvActivity.sendActivityUpdate()
13997

14098
await assertMessagesShown([
141-
['Your CodeCatalyst Dev Environment has been inactive for 4 minutes, and will stop in 5 minutes.', 4],
142-
['Your CodeCatalyst Dev Environment has been inactive for 5 minutes, and will stop in 4 minutes.', 5],
143-
['Your CodeCatalyst Dev Environment has been inactive for 6 minutes, and will stop in 3 minutes.', 6],
144-
['Your CodeCatalyst Dev Environment has been inactive for 7 minutes, and will stop in 2 minutes.', 7],
145-
['Your CodeCatalyst Dev Environment has been inactive for 8 minutes, and will stop soon.', 8],
99+
['Your CodeCatalyst Dev Environment has been inactive for 4 minutes, and will stop soon.', 4],
146100
])
147101
})
148102

149-
it('does not show any inactivity message if a newer user activity is found using the api', async function () {
103+
it('does not show inactivity message if user activity is found using the API', async function () {
150104
// This gets checked each time before we decide to show the message.
151105
// If a new user activity exists then we abort showing the message.
152106
devEnvActivity.isLocalActivityStale.callsFake(async () => {
@@ -191,22 +145,6 @@ describe('InactivityMessages', function () {
191145
}
192146
}
193147

194-
/**
195-
* Starts capturing all vscode messages shown and records them in {@link actualMessages}.
196-
*
197-
* The `minute` field in {@link actualMessages} records the minute the message was shown.
198-
* This value is relative to {@link relativeMinuteMillis}.
199-
*/
200-
function startCapturingMessages() {
201-
const start = Date.now()
202-
const messages: { message: string; minute: number }[] = []
203-
testWindow.onDidShowMessage(async message => {
204-
const now = Date.now()
205-
messages.push({ message: message.message, minute: Math.floor((now - start) / relativeMinuteMillis) })
206-
})
207-
actualMessages = messages
208-
}
209-
210148
let _initialOffset = 0
211149
/**
212150
* This is used for the edge case where the MDE was previously updated with an activity
@@ -219,8 +157,26 @@ describe('InactivityMessages', function () {
219157
function getLatestTimestamp() {
220158
let timestamp = Date.now()
221159
timestamp -= _initialOffset
222-
_initialOffset = 0
223160

224161
return timestamp
225162
}
163+
164+
/**
165+
* Starts capturing all vscode messages shown and records them in {@link actualMessages}.
166+
*
167+
* The `minute` field in {@link actualMessages} records the minute the message was shown.
168+
* This value is relative to {@link relativeMinuteMillis}.
169+
*/
170+
function startCapturingMessages() {
171+
const start = Date.now()
172+
const messages: { message: string; minute: number }[] = []
173+
testWindow.onDidShowMessage(async message => {
174+
const now = Date.now()
175+
messages.push({
176+
message: message.message,
177+
minute: Math.floor((now + _initialOffset - start) / relativeMinuteMillis),
178+
})
179+
})
180+
actualMessages = messages
181+
}
226182
})
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
{
2+
"type": "Feature",
3+
"description": "CodeCatalyst: always show Dev Environment timeout warning as modal prompt"
4+
}

0 commit comments

Comments
 (0)