Skip to content

Commit f87f941

Browse files
EhabYEhabY
committed
Review comments and rebase fallout
1 parent 3ac0272 commit f87f941

File tree

3 files changed

+96
-71
lines changed

3 files changed

+96
-71
lines changed

‎CHANGELOG.md‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66

77
- Always enable verbose (`-v`) flag when a log directory is configured (`coder.proxyLogDir`).
88
- Automatically start a workspace without prompting if it is explicitly opened but not running.
9-
- Replaced SSE paths with a one-way WebSocket and centralized creation on `OneWayWebSocket`, and unified socket handling.
109

1110
### Added
1211

‎src/api/coderApi.ts‎

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ import { getHeaderCommand, getHeaders } from "../headers";
1414
import {
1515
createRequestMeta,
1616
logRequest,
17-
logRequestError,
17+
logError,
1818
logResponse,
1919
} from "../logging/httpLogger";
2020
import { Logger } from "../logging/logger";
@@ -211,27 +211,24 @@ function addLoggingInterceptors(
211211
) {
212212
client.interceptors.request.use(
213213
(config) => {
214-
const meta = createRequestMeta();
215-
(config as RequestConfigWithMeta).metadata = meta;
216-
logRequest(logger, meta.requestId, config, getLogLevel(configProvider()));
214+
const configWithMeta = config as RequestConfigWithMeta;
215+
configWithMeta.metadata = createRequestMeta();
216+
logRequest(logger, configWithMeta, getLogLevel(configProvider()));
217217
return config;
218218
},
219219
(error: unknown) => {
220-
logRequestError(logger, error);
220+
logError(logger, error, getLogLevel(configProvider()));
221221
return Promise.reject(error);
222222
},
223223
);
224224

225225
client.interceptors.response.use(
226226
(response) => {
227-
const meta = (response.config as RequestConfigWithMeta).metadata;
228-
if (meta) {
229-
logResponse(logger, meta, response, getLogLevel(configProvider()));
230-
}
227+
logResponse(logger, response, getLogLevel(configProvider()));
231228
return response;
232229
},
233230
(error: unknown) => {
234-
logRequestError(logger, error);
231+
logError(logger, error, getLogLevel(configProvider()));
235232
return Promise.reject(error);
236233
},
237234
);

‎src/logging/httpLogger.ts‎

Lines changed: 89 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,4 @@
1-
import type {
2-
AxiosError,
3-
AxiosResponse,
4-
InternalAxiosRequestConfig,
5-
} from "axios";
1+
import type { AxiosError, AxiosResponse } from "axios";
62
import { isAxiosError } from "axios";
73
import { getErrorMessage } from "coder/site/src/api/errors";
84
import { getErrorDetail } from "../error";
@@ -22,109 +18,142 @@ import {
2218
} from "./types";
2319
import { createRequestId, shortId } from "./utils";
2420

21+
/**
22+
* Creates metadata for tracking HTTP requests.
23+
*/
2524
export function createRequestMeta(): RequestMeta {
2625
return {
2726
requestId: createRequestId(),
2827
startedAt: Date.now(),
2928
};
3029
}
3130

31+
/**
32+
* Logs an outgoing HTTP RESTful request.
33+
*/
3234
export function logRequest(
3335
logger: Logger,
34-
requestId: string,
35-
config: InternalAxiosRequestConfig,
36+
config: RequestConfigWithMeta,
3637
logLevel: HttpClientLogLevel,
3738
): void {
3839
if (logLevel === HttpClientLogLevel.NONE) {
3940
return;
4041
}
4142

42-
const method = formatMethod(config.method);
43-
const url = formatUri(config);
43+
const { requestId, method, url } = parseConfig(config);
4444
const len = formatContentLength(config.headers, config.data);
4545

46-
let msg = `→ ${shortId(requestId)} ${method} ${url} ${len}`;
47-
if (logLevel >= HttpClientLogLevel.HEADERS) {
48-
msg += `\n${formatHeaders(config.headers)}`;
49-
}
50-
51-
if (logLevel >= HttpClientLogLevel.BODY) {
52-
msg += `\n${formatBody(config.data)}`;
53-
}
54-
55-
logger.trace(msg);
46+
const msg = [
47+
`→ ${shortId(requestId)} ${method} ${url} ${len}`,
48+
...buildExtraLogs(config.headers, config.data, logLevel),
49+
];
50+
logger.trace(msg.join("\n"));
5651
}
5752

53+
/**
54+
* Logs an incoming HTTP RESTful response.
55+
*/
5856
export function logResponse(
5957
logger: Logger,
60-
meta: RequestMeta,
6158
response: AxiosResponse,
6259
logLevel: HttpClientLogLevel,
6360
): void {
6461
if (logLevel === HttpClientLogLevel.NONE) {
6562
return;
6663
}
6764

68-
const method = formatMethod(response.config.method);
69-
const url = formatUri(response.config);
70-
const time = formatTime(Date.now() - meta.startedAt);
65+
const { requestId, method, url, time } = parseConfig(response.config);
7166
const len = formatContentLength(response.headers, response.data);
7267

73-
let msg = `← ${shortId(meta.requestId)} ${response.status} ${method} ${url} ${len} ${time}`;
74-
75-
if (logLevel >= HttpClientLogLevel.HEADERS) {
76-
msg += `\n${formatHeaders(response.headers)}`;
77-
}
78-
79-
if (logLevel >= HttpClientLogLevel.BODY) {
80-
msg += `\n${formatBody(response.data)}`;
81-
}
82-
83-
logger.trace(msg);
68+
const msg = [
69+
`← ${shortId(requestId)} ${response.status} ${method} ${url} ${len} ${time}`,
70+
...buildExtraLogs(response.headers, response.data, logLevel),
71+
];
72+
logger.trace(msg.join("\n"));
8473
}
8574

86-
export function logRequestError(
75+
/**
76+
* Logs HTTP RESTful request errors and failures.
77+
*
78+
* Note: Errors are always logged regardless of log level.
79+
*/
80+
export function logError(
8781
logger: Logger,
8882
error: AxiosError | unknown,
83+
logLevel: HttpClientLogLevel,
8984
): void {
9085
if (isAxiosError(error)) {
9186
const config = error.config as RequestConfigWithMeta | undefined;
92-
const meta = config?.metadata;
93-
const method = formatMethod(config?.method);
94-
const url = formatUri(config);
95-
const requestId = meta?.requestId || "unknown";
96-
const time = meta ? formatTime(Date.now() - meta.startedAt) : "?ms";
87+
const { requestId, method, url, time } = parseConfig(config);
9788

98-
const msg = getErrorMessage(error, "");
89+
const errMsg = getErrorMessage(error, "");
9990
const detail = getErrorDetail(error) ?? "";
91+
const errorParts = [errMsg, detail]
92+
.map((part) => part.trim())
93+
.filter(Boolean);
10094

95+
let logPrefix: string;
96+
let extraLines: string[];
10197
if (error.response) {
102-
const msgParts = [
103-
`← ${shortId(requestId)} ${error.response.status} ${method} ${url} ${time}`,
104-
msg,
105-
detail,
106-
];
107-
if (msg.trim().length === 0 && detail.trim().length === 0) {
108-
const responseData =
98+
if (errorParts.length === 0) {
99+
errorParts.push(
109100
error.response.statusText ||
110-
String(error.response.data).slice(0, 100) ||
111-
"No error info";
112-
msgParts.push(responseData);
101+
String(error.response.data).slice(0, 100) ||
102+
"No error info",
103+
);
113104
}
114105

115-
const fullMsg = msgParts.map((str) => str.trim()).join(" - ");
116-
const headers = formatHeaders(error.response.headers);
117-
logger.error(`${fullMsg}\n${headers}`, error);
106+
logPrefix = `← ${shortId(requestId)} ${error.response.status} ${method} ${url} ${time}`;
107+
extraLines = buildExtraLogs(
108+
error.response.headers,
109+
error.response.data,
110+
logLevel,
111+
);
118112
} else {
119-
const reason = error.code || error.message || "Network error";
120-
const errorInfo = [msg, detail, reason].filter(Boolean).join(" - ");
121-
const headers = formatHeaders(config?.headers ?? {});
122-
logger.error(
123-
`✗ ${shortId(requestId)} ${method} ${url} ${time} - ${errorInfo}\n${headers}`,
124-
error,
113+
if (errorParts.length === 0) {
114+
errorParts.push(error.code || "Network error");
115+
}
116+
logPrefix = `✗ ${shortId(requestId)} ${method} ${url} ${time}`;
117+
extraLines = buildExtraLogs(
118+
error?.config?.headers ?? {},
119+
error.config?.data,
120+
logLevel,
125121
);
126122
}
123+
124+
const msg = [[logPrefix, ...errorParts].join(" - "), ...extraLines];
125+
logger.error(msg.join("\n"));
127126
} else {
128127
logger.error("Request error", error);
129128
}
130129
}
130+
131+
function buildExtraLogs(
132+
headers: Record<string, unknown>,
133+
body: unknown,
134+
logLevel: HttpClientLogLevel,
135+
) {
136+
const msg = [];
137+
if (logLevel >= HttpClientLogLevel.HEADERS) {
138+
msg.push(formatHeaders(headers));
139+
}
140+
if (logLevel >= HttpClientLogLevel.BODY) {
141+
msg.push(formatBody(body));
142+
}
143+
return msg;
144+
}
145+
146+
function parseConfig(config: RequestConfigWithMeta | undefined): {
147+
requestId: string;
148+
method: string;
149+
url: string;
150+
time: string;
151+
} {
152+
const meta = config?.metadata;
153+
return {
154+
requestId: meta?.requestId || "unknown",
155+
method: formatMethod(config?.method),
156+
url: formatUri(config),
157+
time: meta ? formatTime(Date.now() - meta.startedAt) : "?ms",
158+
};
159+
}

0 commit comments

Comments
 (0)