Skip to content

Commit 90e645a

Browse files
plemarquandplemarquand
committed
Address feedback
1 parent 8fcd96a commit 90e645a

File tree

3 files changed

+111
-62
lines changed

3 files changed

+111
-62
lines changed

‎src/WorkspaceContext.ts‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -475,12 +475,7 @@ export class WorkspaceContext implements vscode.Disposable {
475475
);
476476

477477
if (this.getActiveWorkspaceFolder(vscode.window.activeTextEditor) === workspaceFolder) {
478-
const focusStartTime = Date.now();
479478
await this.focusTextEditor(vscode.window.activeTextEditor);
480-
const focusElapsed = Date.now() - focusStartTime;
481-
this.logger.info(
482-
`Focus text editor for ${workspaceFolder.name} completed in ${focusElapsed}ms`
483-
);
484479
}
485480
}
486481

‎src/logging/FileTransport.ts‎

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
//===----------------------------------------------------------------------===//
2+
//
3+
// This source file is part of the VS Code Swift open source project
4+
//
5+
// Copyright (c) 2024 the VS Code Swift project authors
6+
// Licensed under Apache License v2.0
7+
//
8+
// See LICENSE.txt for license information
9+
// See CONTRIBUTORS.txt for the list of VS Code Swift project authors
10+
//
11+
// SPDX-License-Identifier: Apache-2.0
12+
//
13+
//===----------------------------------------------------------------------===//
14+
import * as fs from "fs";
15+
import * as path from "path";
16+
import * as TransportType from "winston-transport";
17+
18+
// Compile error if don't use "require": https://github.com/swiftlang/vscode-swift/actions/runs/16529946578/job/46752753379?pr=1746
19+
// eslint-disable-next-line @typescript-eslint/no-require-imports
20+
const Transport: typeof TransportType = require("winston-transport");
21+
22+
export class FileTransport extends Transport {
23+
private fileHandle: fs.WriteStream | null = null;
24+
private pendingLogs: string[] = [];
25+
private isReady = false;
26+
27+
constructor(private readonly filePath: string) {
28+
super();
29+
this.initializeFile();
30+
}
31+
32+
private initializeFile(): void {
33+
try {
34+
// Ensure the directory exists
35+
const dir = path.dirname(this.filePath);
36+
if (!fs.existsSync(dir)) {
37+
fs.mkdirSync(dir, { recursive: true });
38+
}
39+
40+
// Create write stream
41+
this.fileHandle = fs.createWriteStream(this.filePath, { flags: "a" });
42+
43+
this.fileHandle.on("ready", () => {
44+
this.isReady = true;
45+
this.flushPendingLogs();
46+
});
47+
48+
this.fileHandle.on("error", error => {
49+
// eslint-disable-next-line no-console
50+
console.error(`FileTransport error: ${error}`);
51+
this.isReady = false;
52+
});
53+
} catch (error) {
54+
// eslint-disable-next-line no-console
55+
console.error(`Failed to initialize FileTransport: ${error}`);
56+
this.isReady = false;
57+
}
58+
}
59+
60+
private flushPendingLogs(): void {
61+
if (this.fileHandle && this.pendingLogs.length > 0) {
62+
for (const log of this.pendingLogs) {
63+
this.fileHandle.write(log + "\n");
64+
}
65+
this.pendingLogs = [];
66+
}
67+
}
68+
69+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
70+
public log(info: any, next: () => void): void {
71+
// Get the formatted message from winston
72+
const logMessage = info[Symbol.for("message")];
73+
74+
if (this.isReady && this.fileHandle) {
75+
this.fileHandle.write(logMessage + "\n");
76+
} else {
77+
// Buffer logs if file isn't ready yet
78+
this.pendingLogs.push(logMessage);
79+
}
80+
81+
next();
82+
}
83+
84+
public close(): void {
85+
if (this.fileHandle) {
86+
this.fileHandle.end();
87+
this.fileHandle = null;
88+
}
89+
this.isReady = false;
90+
this.pendingLogs = [];
91+
}
92+
}

‎src/logging/SwiftLogger.ts‎

Lines changed: 19 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -11,12 +11,12 @@
1111
// SPDX-License-Identifier: Apache-2.0
1212
//
1313
//===----------------------------------------------------------------------===//
14-
import * as fs from "fs";
1514
import * as vscode from "vscode";
1615
import * as winston from "winston";
1716

1817
import configuration from "../configuration";
1918
import { IS_RUNNING_IN_DEVELOPMENT_MODE, IS_RUNNING_UNDER_TEST } from "../utilities/utilities";
19+
import { FileTransport } from "./FileTransport";
2020
import { OutputChannelTransport } from "./OutputChannelTransport";
2121
import { RollingLog } from "./RollingLog";
2222
import { RollingLogTransport } from "./RollingLogTransport";
@@ -32,8 +32,8 @@ export class SwiftLogger implements vscode.Disposable {
3232
private logger: winston.Logger;
3333
protected rollingLog: RollingLog;
3434
protected outputChannel: vscode.OutputChannel;
35-
private fileTransportReady = false;
36-
private pendingLogs: Array<{ level: string; message: string; meta?: LoggerMeta }> = [];
35+
private fileTransport: FileTransport;
36+
private cachedOutputChannelLevel: string | undefined;
3737

3838
constructor(
3939
public readonly name: string,
@@ -46,9 +46,14 @@ export class SwiftLogger implements vscode.Disposable {
4646
ouptutChannelTransport.level = this.outputChannelLevel;
4747
const rollingLogTransport = new RollingLogTransport(this.rollingLog);
4848

49-
// Create logger with minimal transports initially for faster startup
50-
const initialTransports = [
49+
// Create file transport
50+
this.fileTransport = new FileTransport(this.logFilePath);
51+
this.fileTransport.level = "debug"; // File logging at the 'debug' level always
52+
53+
// Create logger with all transports
54+
const transports = [
5155
ouptutChannelTransport,
56+
this.fileTransport,
5257
// Only want to capture the rolling log in memory when testing
5358
...(IS_RUNNING_UNDER_TEST ? [rollingLogTransport] : []),
5459
...(IS_RUNNING_IN_DEVELOPMENT_MODE
@@ -57,7 +62,7 @@ export class SwiftLogger implements vscode.Disposable {
5762
];
5863

5964
this.logger = winston.createLogger({
60-
transports: initialTransports,
65+
transports: transports,
6166
format: winston.format.combine(
6267
winston.format.errors({ stack: true }),
6368
winston.format.timestamp({ format: "YYYY-MM-DD HH:mm:ss.SSS" }), // This is the format of `vscode.LogOutputChannel`
@@ -67,43 +72,6 @@ export class SwiftLogger implements vscode.Disposable {
6772
winston.format.colorize()
6873
),
6974
});
70-
71-
// Add file transport asynchronously to avoid blocking startup
72-
setTimeout(() => {
73-
try {
74-
const fileTransport = new winston.transports.File({
75-
filename: this.logFilePath,
76-
level: "debug", // File logging at the 'debug' level always
77-
});
78-
79-
// Add the file transport to the main logger
80-
this.logger.add(fileTransport);
81-
this.fileTransportReady = true;
82-
83-
// Write buffered logs directly to the file using Node.js fs
84-
if (this.pendingLogs.length > 0) {
85-
const logEntries =
86-
this.pendingLogs
87-
.map(({ level, message }) => {
88-
const timestamp = new Date()
89-
.toISOString()
90-
.replace("T", " ")
91-
.replace("Z", "");
92-
return `${timestamp} [${level}] ${message}`;
93-
})
94-
.join("\n") + "\n";
95-
96-
fs.appendFileSync(this.logFilePath, logEntries);
97-
}
98-
99-
this.pendingLogs = []; // Clear the buffer
100-
} catch (error) {
101-
// If file transport fails, continue with output channel only
102-
this.logger.warn(`Failed to initialize file logging: ${error}`);
103-
this.fileTransportReady = true; // Mark as ready even if failed to stop buffering
104-
this.pendingLogs = []; // Clear the buffer
105-
}
106-
}, 0);
10775
this.disposables.push(
10876
{
10977
dispose: () => {
@@ -114,6 +82,7 @@ export class SwiftLogger implements vscode.Disposable {
11482
if (rollingLogTransport.close) {
11583
rollingLogTransport.close();
11684
}
85+
this.fileTransport.close();
11786
},
11887
},
11988
vscode.workspace.onDidChangeConfiguration(e => {
@@ -122,7 +91,7 @@ export class SwiftLogger implements vscode.Disposable {
12291
e.affectsConfiguration("swift.diagnostics")
12392
) {
12493
// Clear cache when configuration changes
125-
this._cachedOutputChannelLevel = undefined;
94+
this.cachedOutputChannelLevel = undefined;
12695
ouptutChannelTransport.level = this.outputChannelLevel;
12796
}
12897
})
@@ -154,17 +123,12 @@ export class SwiftLogger implements vscode.Disposable {
154123
}
155124

156125
private logWithBuffer(level: string, message: string | Error, meta?: LoggerMeta) {
157-
// Always log to current transports (output channel, console, etc.)
126+
// Log to all transports (output channel, file, console, etc.)
158127
if (message instanceof Error) {
159128
this.logger.log(level, message);
160129
} else {
161130
this.logger.log(level, message, meta);
162131
}
163-
164-
// If file transport isn't ready yet, buffer the log for replay
165-
if (!this.fileTransportReady) {
166-
this.pendingLogs.push({ level, message: message.toString(), meta });
167-
}
168132
}
169133

170134
get logs(): string[] {
@@ -193,25 +157,23 @@ export class SwiftLogger implements vscode.Disposable {
193157
return fullMessage;
194158
}
195159

196-
private _cachedOutputChannelLevel: string | undefined;
197-
198160
private get outputChannelLevel(): string {
199161
// Cache the configuration lookup to avoid repeated expensive calls during initialization
200-
if (this._cachedOutputChannelLevel === undefined) {
162+
if (this.cachedOutputChannelLevel === undefined) {
201163
const info = vscode.workspace
202164
.getConfiguration("swift")
203165
.inspect("outputChannelLogLevel");
204166
// If the user has explicitly set `outputChannelLogLevel` then use it, otherwise
205167
// check the deprecated `diagnostics` property
206168
if (info?.globalValue || info?.workspaceValue || info?.workspaceFolderValue) {
207-
this._cachedOutputChannelLevel = configuration.outputChannelLogLevel;
169+
this.cachedOutputChannelLevel = configuration.outputChannelLogLevel;
208170
} else if (configuration.diagnostics) {
209-
this._cachedOutputChannelLevel = "debug";
171+
this.cachedOutputChannelLevel = "debug";
210172
} else {
211-
this._cachedOutputChannelLevel = configuration.outputChannelLogLevel;
173+
this.cachedOutputChannelLevel = configuration.outputChannelLogLevel;
212174
}
213175
}
214-
return this._cachedOutputChannelLevel;
176+
return this.cachedOutputChannelLevel;
215177
}
216178

217179
dispose() {

0 commit comments

Comments
 (0)