Skip to content

Commit d8beda6

Browse files
EhabYEhabY
committed
Review comments
1 parent bc4ba72 commit d8beda6

File tree

3 files changed

+60
-85
lines changed

3 files changed

+60
-85
lines changed

‎src/remote/remote.ts‎

Lines changed: 11 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -291,8 +291,8 @@ export class Remote {
291291
this.vscodeProposed,
292292
this.contextManager,
293293
);
294-
disposables.push(monitor);
295294
disposables.push(
295+
monitor,
296296
monitor.onChange.event((w) => (this.commands.workspace = w)),
297297
);
298298

@@ -310,7 +310,7 @@ export class Remote {
310310
disposables.push(stateMachine);
311311

312312
try {
313-
await this.vscodeProposed.window.withProgress(
313+
workspace = await this.vscodeProposed.window.withProgress(
314314
{
315315
location: vscode.ProgressLocation.Notification,
316316
cancellable: false,
@@ -320,10 +320,8 @@ export class Remote {
320320
let inProgress = false;
321321
let pendingWorkspace: Workspace | null = null;
322322

323-
await new Promise<void>((resolve, reject) => {
323+
return new Promise<Workspace>((resolve, reject) => {
324324
const processWorkspace = async (w: Workspace) => {
325-
workspace = w;
326-
327325
if (inProgress) {
328326
// Process one workspace at a time, keeping only the last
329327
pendingWorkspace = w;
@@ -340,26 +338,19 @@ export class Remote {
340338
);
341339
if (isReady) {
342340
subscription.dispose();
343-
resolve();
341+
resolve(w);
344342
return;
345343
}
346-
347-
if (pendingWorkspace) {
348-
const isReadyAfter = await stateMachine.processWorkspace(
349-
pendingWorkspace,
350-
progress,
351-
);
352-
if (isReadyAfter) {
353-
subscription.dispose();
354-
resolve();
355-
}
356-
}
357344
} catch (error) {
358345
subscription.dispose();
359346
reject(error);
360347
} finally {
361348
inProgress = false;
362349
}
350+
351+
if (pendingWorkspace) {
352+
processWorkspace(pendingWorkspace);
353+
}
363354
};
364355

365356
processWorkspace(workspace);
@@ -373,6 +364,9 @@ export class Remote {
373364
stateMachine.dispose();
374365
}
375366

367+
// Mark initial setup as complete so the monitor can start notifying about state changes
368+
monitor.markInitialSetupComplete();
369+
376370
const agents = extractAgents(workspace.latest_build.resources);
377371
const agent = agents.find(
378372
(agent) => agent.id === stateMachine.getAgentId(),

‎src/remote/workspaceStateMachine.ts‎

Lines changed: 38 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -32,10 +32,7 @@ export class WorkspaceStateMachine implements vscode.Disposable {
3232

3333
private agentId: string | undefined;
3434

35-
private buildLogSocket: {
36-
socket: OneWayWebSocket<ProvisionerJobLog> | null;
37-
buildId: string | null;
38-
} = { socket: null, buildId: null };
35+
private buildLogSocket: OneWayWebSocket<ProvisionerJobLog> | null = null;
3936

4037
private agentLogSocket: OneWayWebSocket<WorkspaceAgentLog[]> | null = null;
4138

@@ -58,7 +55,7 @@ export class WorkspaceStateMachine implements vscode.Disposable {
5855
*/
5956
async processWorkspace(
6057
workspace: Workspace,
61-
progress?: vscode.Progress<{ message?: string }>,
58+
progress: vscode.Progress<{ message?: string }>,
6259
): Promise<boolean> {
6360
const workspaceName = createWorkspaceIdentifier(workspace);
6461

@@ -75,7 +72,7 @@ export class WorkspaceStateMachine implements vscode.Disposable {
7572
throw new Error(`User declined to start ${workspaceName}`);
7673
}
7774

78-
progress?.report({ message: `Starting ${workspaceName}...` });
75+
progress.report({ message: `Starting ${workspaceName}...` });
7976
this.logger.info(`Starting ${workspaceName}...`);
8077
const globalConfigDir = this.pathResolver.getGlobalConfigDir(
8178
this.parts.label,
@@ -95,20 +92,19 @@ export class WorkspaceStateMachine implements vscode.Disposable {
9592
case "pending":
9693
case "starting":
9794
case "stopping":
98-
progress?.report({ message: "Waiting for workspace build..." });
95+
// Clear the agent ID since it could change after a restart
96+
this.agentId = undefined;
97+
this.closeAgentLogSocket();
98+
progress.report({
99+
message: `Waiting for workspace build (${workspace.latest_build.status})...`,
100+
});
99101
this.logger.info(`Waiting for ${workspaceName}...`);
100102

101-
if (!this.buildLogSocket.socket) {
102-
const socket = await streamBuildLogs(
103-
this.workspaceClient,
104-
this.terminal.writeEmitter,
105-
workspace,
106-
);
107-
this.buildLogSocket = {
108-
socket,
109-
buildId: workspace.latest_build.id,
110-
};
111-
}
103+
this.buildLogSocket ??= await streamBuildLogs(
104+
this.workspaceClient,
105+
this.terminal.writeEmitter,
106+
workspace,
107+
);
112108
return false;
113109

114110
case "deleted":
@@ -117,12 +113,6 @@ export class WorkspaceStateMachine implements vscode.Disposable {
117113
case "canceling":
118114
this.closeBuildLogSocket();
119115
throw new Error(`${workspaceName} is ${workspace.latest_build.status}`);
120-
121-
default:
122-
this.closeBuildLogSocket();
123-
throw new Error(
124-
`${workspaceName} unknown status: ${workspace.latest_build.status}`,
125-
);
126116
}
127117

128118
const agents = extractAgents(workspace.latest_build.resources);
@@ -144,33 +134,30 @@ export class WorkspaceStateMachine implements vscode.Disposable {
144134
throw new Error(`Agent not found in ${workspaceName} resources`);
145135
}
146136

137+
const agentName = `${workspaceName}/${agent.name}`;
138+
147139
switch (agent.status) {
148140
case "connecting":
149-
progress?.report({
150-
message: `Waiting for agent ${agent.name} to connect...`,
141+
progress.report({
142+
message: `Waiting for agent ${agentName} to connect...`,
151143
});
152-
this.logger.debug(`Waiting for agent ${agent.name}...`);
144+
this.logger.debug(`Waiting for agent ${agentName}...`);
153145
return false;
154146

155147
case "disconnected":
156-
throw new Error(`${workspaceName}/${agent.name} disconnected`);
148+
throw new Error(`${agentName} disconnected`);
157149

158150
case "timeout":
159-
progress?.report({
160-
message: `Agent ${agent.name} timed out, continuing to wait...`,
151+
progress.report({
152+
message: `Agent ${agentName} timed out, continuing to wait...`,
161153
});
162154
this.logger.debug(
163-
`Agent ${agent.name} timed out, continuing to wait...`,
155+
`Agent ${agentName} timed out, continuing to wait...`,
164156
);
165157
return false;
166158

167159
case "connected":
168160
break;
169-
170-
default:
171-
throw new Error(
172-
`${workspaceName}/${agent.name} unknown status: ${agent.status}`,
173-
);
174161
}
175162

176163
switch (agent.lifecycle_state) {
@@ -186,10 +173,10 @@ export class WorkspaceStateMachine implements vscode.Disposable {
186173
return true;
187174
}
188175

189-
progress?.report({
190-
message: `Waiting for agent ${agent.name} startup scripts...`,
176+
progress.report({
177+
message: `Waiting for ${agentName} startup scripts...`,
191178
});
192-
this.logger.debug(`Waiting for agent ${agent.name} startup scripts...`);
179+
this.logger.debug(`Waiting for ${agentName} startup scripts...`);
193180

194181
this.agentLogSocket ??= await streamAgentLogs(
195182
this.workspaceClient,
@@ -200,44 +187,41 @@ export class WorkspaceStateMachine implements vscode.Disposable {
200187
}
201188

202189
case "created":
203-
progress?.report({
204-
message: `Waiting for agent ${agent.name} to start...`,
190+
progress.report({
191+
message: `Waiting for ${agentName} to start...`,
205192
});
206-
this.logger.debug(
207-
`Waiting for ${workspaceName}/${agent.name} to start...`,
208-
);
193+
this.logger.debug(`Waiting for ${agentName} to start...`);
209194
return false;
210195

211196
case "start_error":
212197
this.closeAgentLogSocket();
213198
this.logger.info(
214-
`Agent ${agent.name} startup script failed, but continuing...`,
199+
`Agent ${agentName} startup script failed, but continuing...`,
215200
);
216201
return true;
217202

218203
case "start_timeout":
219204
this.closeAgentLogSocket();
220205
this.logger.info(
221-
`Agent ${agent.name} startup script timed out, but continuing...`,
206+
`Agent ${agentName} startup script timed out, but continuing...`,
222207
);
223208
return true;
224209

210+
case "shutting_down":
225211
case "off":
226-
this.closeAgentLogSocket();
227-
throw new Error(`${workspaceName}/${agent.name} is off`);
228-
229-
default:
212+
case "shutdown_error":
213+
case "shutdown_timeout":
230214
this.closeAgentLogSocket();
231215
throw new Error(
232-
`${workspaceName}/${agent.name} unknown lifecycle state: ${agent.lifecycle_state}`,
216+
`Invalid lifecycle state '${agent.lifecycle_state}' for ${agentName}`,
233217
);
234218
}
235219
}
236220

237221
private closeBuildLogSocket(): void {
238-
if (this.buildLogSocket.socket) {
239-
this.buildLogSocket.socket.close();
240-
this.buildLogSocket = { socket: null, buildId: null };
222+
if (this.buildLogSocket) {
223+
this.buildLogSocket.close();
224+
this.buildLogSocket = null;
241225
}
242226
}
243227

‎src/workspace/workspaceMonitor.ts‎

Lines changed: 11 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ export class WorkspaceMonitor implements vscode.Disposable {
2929
private notifiedDeletion = false;
3030
private notifiedOutdated = false;
3131
private notifiedNotRunning = false;
32-
private isReady = false;
32+
private completedInitialSetup = false;
3333

3434
readonly onChange = new vscode.EventEmitter<Workspace>();
3535
private readonly statusBarItem: vscode.StatusBarItem;
@@ -111,6 +111,10 @@ export class WorkspaceMonitor implements vscode.Disposable {
111111
return monitor;
112112
}
113113

114+
public markInitialSetupComplete(): void {
115+
this.completedInitialSetup = true;
116+
}
117+
114118
/**
115119
* Permanently close the websocket.
116120
*/
@@ -124,17 +128,16 @@ export class WorkspaceMonitor implements vscode.Disposable {
124128
}
125129

126130
private update(workspace: Workspace) {
127-
this.updateReadyState(workspace);
128131
this.updateContext(workspace);
129132
this.updateStatusBar(workspace);
130133
}
131134

132135
private maybeNotify(workspace: Workspace) {
133136
this.maybeNotifyOutdated(workspace);
134137
this.maybeNotifyAutostop(workspace);
135-
this.maybeNotifyDeletion(workspace);
136-
if (this.isReady) {
138+
if (this.completedInitialSetup) {
137139
// This instance might be created before the workspace is running
140+
this.maybeNotifyDeletion(workspace);
138141
this.maybeNotifyNotRunning(workspace);
139142
}
140143
}
@@ -198,7 +201,7 @@ export class WorkspaceMonitor implements vscode.Disposable {
198201
}
199202

200203
private isImpending(target: string, notifyTime: number): boolean {
201-
const nowTime = new Date().getTime();
204+
const nowTime = Date.now();
202205
const targetTime = new Date(target).getTime();
203206
const timeLeft = targetTime - nowTime;
204207
return timeLeft >= 0 && timeLeft <= notifyTime;
@@ -249,21 +252,15 @@ export class WorkspaceMonitor implements vscode.Disposable {
249252
this.logger.error(message);
250253
}
251254

252-
private updateReadyState(workspace: Workspace): void {
253-
if (workspace.latest_build.status === "running") {
254-
this.isReady = true;
255-
}
256-
}
257-
258255
private updateContext(workspace: Workspace) {
259256
this.contextManager.set("coder.workspace.updatable", workspace.outdated);
260257
}
261258

262259
private updateStatusBar(workspace: Workspace) {
263-
if (!workspace.outdated) {
264-
this.statusBarItem.hide();
265-
} else {
260+
if (workspace.outdated) {
266261
this.statusBarItem.show();
262+
} else {
263+
this.statusBarItem.hide();
267264
}
268265
}
269266
}

0 commit comments

Comments
 (0)