Skip to content

Commit 9e4101d

Browse files
committed
fix: drop throughput-based slowness detection, polish tooltip
The download_bytes_sec/upload_bytes_sec fields from the Coder CLI measure actual tunnel traffic during the poll window, not link capacity. Idle SSH sessions always dipped below the 5 Mbps default even on fast networks, producing spurious warnings. Keep latency as the only slowness signal. Display throughput in the tooltip as informational data only. Replace the warning-icon-mid-line layout with a bold header, tight metric rows, and an action row containing explicit Ping workspace and Configure threshold links.
1 parent 36e8672 commit 9e4101d

5 files changed

Lines changed: 147 additions & 349 deletions

File tree

package.json

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -172,16 +172,6 @@
172172
"type": "number",
173173
"default": 200
174174
},
175-
"coder.networkThreshold.downloadMbps": {
176-
"markdownDescription": "Download speed threshold in Mbps. A warning indicator appears in the status bar when download speed drops below this value. Set to `0` to disable.",
177-
"type": "number",
178-
"default": 5
179-
},
180-
"coder.networkThreshold.uploadMbps": {
181-
"markdownDescription": "Upload speed threshold in Mbps. A warning indicator appears in the status bar when upload speed drops below this value. Set to `0` to disable.",
182-
"type": "number",
183-
"default": 0
184-
},
185175
"coder.httpClientLogLevel": {
186176
"markdownDescription": "Controls the verbosity of HTTP client logging. This affects what details are logged for each HTTP request and response.",
187177
"type": "string",

src/remote/networkStatus.ts

Lines changed: 47 additions & 125 deletions
Original file line numberDiff line numberDiff line change
@@ -3,152 +3,88 @@ import * as vscode from "vscode";
33

44
import type { NetworkInfo } from "./sshProcess";
55

6-
/** Bytes per second in 1 Mbps */
7-
const BYTES_PER_MBPS = 125_000;
8-
96
/** Number of consecutive polls required to trigger or clear a warning */
107
const WARNING_DEBOUNCE_THRESHOLD = 3;
118

12-
export interface ThresholdViolations {
13-
latency: boolean;
14-
download: boolean;
15-
upload: boolean;
16-
}
17-
18-
const NO_VIOLATIONS: ThresholdViolations = {
19-
latency: false,
20-
download: false,
21-
upload: false,
22-
};
9+
const WARNING_BACKGROUND = new vscode.ThemeColor(
10+
"statusBarItem.warningBackground",
11+
);
2312

24-
export function getThresholdConfig(): {
13+
export interface NetworkThresholds {
2514
latencyMs: number;
26-
downloadMbps: number;
27-
uploadMbps: number;
28-
} {
15+
}
16+
17+
function getThresholdConfig(): NetworkThresholds {
2918
const cfg = vscode.workspace.getConfiguration("coder");
3019
return {
3120
latencyMs: cfg.get<number>("networkThreshold.latencyMs", 200),
32-
downloadMbps: cfg.get<number>("networkThreshold.downloadMbps", 5),
33-
uploadMbps: cfg.get<number>("networkThreshold.uploadMbps", 0),
3421
};
3522
}
3623

37-
export function checkThresholdViolations(
24+
export function isLatencySlow(
3825
network: NetworkInfo,
39-
thresholds: { latencyMs: number; downloadMbps: number; uploadMbps: number },
40-
): ThresholdViolations {
41-
return {
42-
latency: thresholds.latencyMs > 0 && network.latency > thresholds.latencyMs,
43-
download:
44-
thresholds.downloadMbps > 0 &&
45-
network.download_bytes_sec / BYTES_PER_MBPS < thresholds.downloadMbps,
46-
upload:
47-
thresholds.uploadMbps > 0 &&
48-
network.upload_bytes_sec / BYTES_PER_MBPS < thresholds.uploadMbps,
49-
};
50-
}
51-
52-
export function hasAnyViolation(violations: ThresholdViolations): boolean {
53-
return violations.latency || violations.download || violations.upload;
54-
}
55-
56-
export function getWarningCommand(violations: ThresholdViolations): string {
57-
const latencyOnly =
58-
violations.latency && !violations.download && !violations.upload;
59-
const throughputOnly =
60-
!violations.latency && (violations.download || violations.upload);
61-
62-
if (latencyOnly) {
63-
return "coder.pingWorkspace";
64-
}
65-
if (throughputOnly) {
66-
return "coder.speedTest";
67-
}
68-
// Multiple types of violations — let the user choose
69-
return "coder.showNetworkDiagnostics";
26+
thresholds: NetworkThresholds,
27+
): boolean {
28+
return thresholds.latencyMs > 0 && network.latency > thresholds.latencyMs;
7029
}
7130

7231
export function buildNetworkTooltip(
7332
network: NetworkInfo,
74-
violations: ThresholdViolations,
75-
thresholds: { latencyMs: number; downloadMbps: number; uploadMbps: number },
33+
latencySlow: boolean,
34+
thresholds: NetworkThresholds,
7635
): vscode.MarkdownString {
7736
const fmt = (bytesPerSec: number) =>
7837
prettyBytes(bytesPerSec * 8, { bits: true }) + "/s";
7938

80-
const lines: string[] = [];
81-
82-
let latencyLine = `Latency: ${network.latency.toFixed(2)}ms`;
83-
if (violations.latency) {
84-
latencyLine += ` $(warning) (threshold: ${thresholds.latencyMs}ms)`;
85-
}
86-
lines.push(latencyLine);
39+
const sections: string[] = [];
8740

88-
let downloadLine = `Download: ${fmt(network.download_bytes_sec)}`;
89-
if (violations.download) {
90-
downloadLine += ` $(warning) (threshold: ${thresholds.downloadMbps} Mbit/s)`;
41+
if (latencySlow) {
42+
sections.push("$(warning) **Slow connection detected**");
9143
}
92-
lines.push(downloadLine);
9344

94-
let uploadLine = `Upload: ${fmt(network.upload_bytes_sec)}`;
95-
if (violations.upload) {
96-
uploadLine += ` $(warning) (threshold: ${thresholds.uploadMbps} Mbit/s)`;
97-
}
98-
lines.push(uploadLine);
45+
const metrics: string[] = [];
46+
metrics.push(
47+
latencySlow
48+
? `Latency: ${network.latency.toFixed(2)}ms (threshold: ${thresholds.latencyMs}ms)`
49+
: `Latency: ${network.latency.toFixed(2)}ms`,
50+
);
51+
metrics.push(`Download: ${fmt(network.download_bytes_sec)}`);
52+
metrics.push(`Upload: ${fmt(network.upload_bytes_sec)}`);
9953

10054
if (network.using_coder_connect) {
101-
lines.push("Connection: Coder Connect");
55+
metrics.push("Connection: Coder Connect");
10256
} else if (network.p2p) {
103-
lines.push("Connection: Direct (P2P)");
57+
metrics.push("Connection: Direct (P2P)");
10458
} else {
105-
lines.push(`Connection: ${network.preferred_derp} (relay)`);
59+
metrics.push(`Connection: ${network.preferred_derp} (relay)`);
10660
}
10761

108-
if (hasAnyViolation(violations)) {
109-
lines.push("");
110-
lines.push(
111-
"_Click for diagnostics_ | [Configure thresholds](command:workbench.action.openSettings?%22coder.networkThreshold%22)",
62+
// Two trailing spaces + \n = hard line break (tight rows within a section).
63+
sections.push(metrics.join(" \n"));
64+
65+
if (latencySlow) {
66+
sections.push(
67+
"[$(pulse) Ping workspace](command:coder.pingWorkspace) · " +
68+
"[$(gear) Configure threshold](command:workbench.action.openSettings?%22coder.networkThreshold%22)",
11269
);
11370
}
11471

115-
const md = new vscode.MarkdownString(lines.join("\n\n"));
72+
// Blank line between sections = paragraph break.
73+
const md = new vscode.MarkdownString(sections.join("\n\n"));
11674
md.isTrusted = true;
11775
md.supportThemeIcons = true;
11876
return md;
11977
}
12078

12179
/**
12280
* Manages network status bar presentation and slowness warning state.
123-
* Owns the warning debounce logic, status bar updates, and the
124-
* diagnostics command registration.
81+
* Owns the warning debounce logic and status bar updates.
12582
*/
126-
export class NetworkStatusReporter implements vscode.Disposable {
83+
export class NetworkStatusReporter {
12784
private warningCounter = 0;
12885
private isWarningActive = false;
129-
private readonly diagnosticsCommand: vscode.Disposable;
130-
131-
constructor(private readonly statusBarItem: vscode.StatusBarItem) {
132-
this.diagnosticsCommand = vscode.commands.registerCommand(
133-
"coder.showNetworkDiagnostics",
134-
async () => {
135-
const pick = await vscode.window.showQuickPick(
136-
[
137-
{ label: "Run Ping", commandId: "coder.pingWorkspace" },
138-
{ label: "Run Speed Test", commandId: "coder.speedTest" },
139-
{
140-
label: "Create Support Bundle",
141-
commandId: "coder.supportBundle",
142-
},
143-
],
144-
{ placeHolder: "Select a diagnostic to run" },
145-
);
146-
if (pick) {
147-
await vscode.commands.executeCommand(pick.commandId);
148-
}
149-
},
150-
);
151-
}
86+
87+
constructor(private readonly statusBarItem: vscode.StatusBarItem) {}
15288

15389
update(network: NetworkInfo, isStale: boolean): void {
15490
let statusText = "$(globe) ";
@@ -166,8 +102,8 @@ export class NetworkStatusReporter implements vscode.Disposable {
166102
}
167103

168104
const thresholds = getThresholdConfig();
169-
const violations = checkThresholdViolations(network, thresholds);
170-
const activeViolations = this.updateWarningState(violations);
105+
const latencySlow = isLatencySlow(network, thresholds);
106+
this.updateWarningState(latencySlow);
171107

172108
if (network.p2p) {
173109
statusText += "Direct ";
@@ -182,32 +118,24 @@ export class NetworkStatusReporter implements vscode.Disposable {
182118
this.statusBarItem.text = statusText;
183119

184120
if (this.isWarningActive) {
185-
this.statusBarItem.backgroundColor = new vscode.ThemeColor(
186-
"statusBarItem.warningBackground",
187-
);
188-
this.statusBarItem.command = getWarningCommand(activeViolations);
121+
this.statusBarItem.backgroundColor = WARNING_BACKGROUND;
122+
this.statusBarItem.command = "coder.pingWorkspace";
189123
} else {
190124
this.statusBarItem.backgroundColor = undefined;
191125
this.statusBarItem.command = undefined;
192126
}
193127

194128
this.statusBarItem.tooltip = buildNetworkTooltip(
195129
network,
196-
activeViolations,
130+
this.isWarningActive,
197131
thresholds,
198132
);
199133

200134
this.statusBarItem.show();
201135
}
202136

203-
/**
204-
* Updates the debounce counter and returns the effective violations
205-
* (current violations when warning is active, all-clear otherwise).
206-
*/
207-
private updateWarningState(
208-
violations: ThresholdViolations,
209-
): ThresholdViolations {
210-
if (hasAnyViolation(violations)) {
137+
private updateWarningState(latencySlow: boolean): void {
138+
if (latencySlow) {
211139
this.warningCounter = Math.min(
212140
this.warningCounter + 1,
213141
WARNING_DEBOUNCE_THRESHOLD,
@@ -221,11 +149,5 @@ export class NetworkStatusReporter implements vscode.Disposable {
221149
} else if (this.warningCounter === 0) {
222150
this.isWarningActive = false;
223151
}
224-
225-
return this.isWarningActive ? violations : NO_VIOLATIONS;
226-
}
227-
228-
dispose(): void {
229-
this.diagnosticsCommand.dispose();
230152
}
231153
}

src/remote/sshProcess.ts

Lines changed: 5 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -255,7 +255,6 @@ export class SshProcessMonitor implements vscode.Disposable {
255255
this.pendingTimeout = undefined;
256256
}
257257
this.statusBarItem.dispose();
258-
this.reporter.dispose();
259258
this._onLogFilePathChange.dispose();
260259
this._onPidChange.dispose();
261260
}
@@ -462,20 +461,15 @@ export class SshProcessMonitor implements vscode.Disposable {
462461

463462
while (!this.disposed && this.currentPid !== undefined) {
464463
const filePath = path.join(networkInfoPath, `${this.currentPid}.json`);
465-
let search: { needed: true; reason: string } | { needed: false } = {
466-
needed: false,
467-
};
464+
let searchReason: string | undefined;
468465

469466
try {
470467
const stats = await fs.stat(filePath);
471468
const ageMs = Date.now() - stats.mtime.getTime();
472469
readFailures = 0;
473470

474471
if (ageMs > staleThreshold) {
475-
search = {
476-
needed: true,
477-
reason: `Network info stale (${Math.round(ageMs / 1000)}s old)`,
478-
};
472+
searchReason = `Network info stale (${Math.round(ageMs / 1000)}s old)`;
479473
} else {
480474
const content = await fs.readFile(filePath, "utf8");
481475
const network = JSON.parse(content) as NetworkInfo;
@@ -488,22 +482,18 @@ export class SshProcessMonitor implements vscode.Disposable {
488482
`Failed to read network info (attempt ${readFailures}): ${(error as Error).message}`,
489483
);
490484
if (readFailures >= maxReadFailures) {
491-
search = {
492-
needed: true,
493-
reason: `Network info missing for ${readFailures} attempts`,
494-
};
485+
searchReason = `Network info missing for ${readFailures} attempts`;
495486
}
496487
}
497488

498-
// Search for new process if needed (with throttling)
499-
if (search.needed) {
489+
if (searchReason !== undefined) {
500490
const timeSinceLastSearch = Date.now() - this.lastStaleSearchTime;
501491
if (timeSinceLastSearch < staleThreshold) {
502492
await this.delay(staleThreshold - timeSinceLastSearch);
503493
continue;
504494
}
505495

506-
logger.debug(`${search.reason}, searching for new SSH process`);
496+
logger.debug(`${searchReason}, searching for new SSH process`);
507497
// searchForProcess will update PID if a different process is found
508498
this.lastStaleSearchTime = Date.now();
509499
await this.searchForProcess();

0 commit comments

Comments
 (0)