Skip to content

Improve error reporting when SourceKit-LSP fails to start #982

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jul 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 42 additions & 13 deletions src/sourcekit-lsp/LanguageClientManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -434,11 +434,14 @@ export class LanguageClientManager {
this.languageClient = undefined;
return;
}
const client = this.createLSPClient(folder);
return this.startClient(client);
const { client, errorHandler } = this.createLSPClient(folder);
return this.startClient(client, errorHandler);
}

private createLSPClient(folder?: vscode.Uri): langclient.LanguageClient {
private createLSPClient(folder?: vscode.Uri): {
client: langclient.LanguageClient;
errorHandler: SourceKitLSPErrorHandler;
} {
const toolchainSourceKitLSP =
this.workspaceContext.toolchain.getToolchainExecutable("sourcekit-lsp");
const lspConfig = configuration.lsp;
Expand Down Expand Up @@ -488,6 +491,7 @@ export class LanguageClientManager {
workspaceFolder = { uri: folder, name: FolderContext.uriName(folder), index: 0 };
}

const errorHandler = new SourceKitLSPErrorHandler(5);
const clientOptions: langclient.LanguageClientOptions = {
documentSelector: LanguageClientManager.documentSelector,
revealOutputChannelOn: langclient.RevealOutputChannelOn.Never,
Expand Down Expand Up @@ -563,21 +567,30 @@ export class LanguageClientManager {
};
})(),
},
errorHandler: new SourceKitLSPErrorHandler(5),
errorHandler,
// Avoid attempting to reinitialize multiple times. If we fail to initialize
// we aren't doing anything different the second time and so will fail again.
initializationFailedHandler: () => false,
initializationOptions: {
"workspace/peekDocuments": true, // workaround for client capability to handle `PeekDocumentsRequest`
},
};

return new langclient.LanguageClient(
"swift.sourcekit-lsp",
"SourceKit Language Server",
serverOptions,
clientOptions
);
return {
client: new langclient.LanguageClient(
"swift.sourcekit-lsp",
"SourceKit Language Server",
serverOptions,
clientOptions
),
errorHandler,
};
}

private async startClient(client: langclient.LanguageClient) {
private async startClient(
client: langclient.LanguageClient,
errorHandler: SourceKitLSPErrorHandler
) {
client.onDidChangeState(e => {
// if state is now running add in any sub-folder workspaces that
// we have cached. If this is the first time we are starting then
Expand All @@ -589,7 +602,6 @@ export class LanguageClientManager {
) {
this.addSubFolderWorkspaces(client);
}
//console.log(e);
});
if (client.clientOptions.workspaceFolder) {
this.workspaceContext.outputChannel.log(
Expand All @@ -609,6 +621,10 @@ export class LanguageClientManager {
this.clientReadyPromise = client
.start()
.then(() => {
// Now that we've started up correctly, start the error handler to auto-restart
// if sourcekit-lsp crashes during normal operation.
errorHandler.enable();

if (this.workspaceContext.swiftVersion.isLessThan(new Version(5, 7, 0))) {
this.legacyInlayHints = activateLegacyInlayHints(client);
}
Expand All @@ -617,7 +633,6 @@ export class LanguageClientManager {
})
.catch(reason => {
this.workspaceContext.outputChannel.log(`${reason}`);
// if language client failed to initialise then shutdown and set to undefined
this.languageClient?.stop();
this.languageClient = undefined;
throw reason;
Expand Down Expand Up @@ -665,10 +680,17 @@ export class LanguageClientManager {
*/
class SourceKitLSPErrorHandler implements langclient.ErrorHandler {
private restarts: number[];
private enabled: boolean = false;

constructor(private maxRestartCount: number) {
this.restarts = [];
}
/**
* Start listening for errors and requesting to restart the LSP server when appropriate.
*/
enable() {
this.enabled = true;
}
/**
* An error has occurred while writing or reading from the connection.
*
Expand All @@ -691,6 +713,13 @@ class SourceKitLSPErrorHandler implements langclient.ErrorHandler {
* The connection to the server got closed.
*/
closed(): langclient.CloseHandlerResult | Promise<langclient.CloseHandlerResult> {
if (!this.enabled) {
return {
action: langclient.CloseAction.DoNotRestart,
handled: true,
};
}

this.restarts.push(Date.now());
if (this.restarts.length <= this.maxRestartCount) {
return { action: langclient.CloseAction.Restart };
Expand Down
16 changes: 11 additions & 5 deletions src/ui/SwiftOutputChannel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@ class RollingLog {
private startIndex: number = 0;
private endIndex: number = 0;
private logCount: number = 0;
private appending: boolean = false;

constructor(private maxLogs: number) {
this._logs = new Array(maxLogs).fill(null);
Expand All @@ -133,23 +134,28 @@ class RollingLog {
}

appendLine(log: string) {
// Writing to a new line that isn't the very first, increment the end index
if (this.logCount > 0) {
this.endIndex = this.incrementIndex(this.endIndex);
}

// We're over the window size, move the start index
if (this.logCount === this.maxLogs) {
this.startIndex = this.incrementIndex(this.startIndex);
} else {
this.logCount++;
}

this._logs[this.endIndex] = log;
this.endIndex = this.incrementIndex(this.endIndex);
}

append(log: string) {
const endIndex = this.endIndex - 1 < 0 ? Math.max(this.logCount - 1, 0) : this.endIndex;
if (this._logs[endIndex] === null) {
if (this.logCount === 0) {
this.logCount = 1;
}
const newLogLine = (this._logs[endIndex] ?? "") + log;
this._logs[endIndex] = newLogLine;
const newLogLine = (this._logs[this.endIndex] ?? "") + log;
this._logs[this.endIndex] = newLogLine;
this.appending = true;
}

replace(log: string) {
Expand Down
17 changes: 17 additions & 0 deletions test/suite/ui/SwiftOutputChannel.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,16 @@ suite("SwiftOutputChannel", function () {
assert.deepEqual(channel.logs, ["b", "cd", "e"]);
});

test("Appends after rolling over", () => {
channel.appendLine("a");
channel.appendLine("b");
channel.appendLine("c");
channel.appendLine("d");
channel.append("e");
channel.appendLine("f");
assert.deepEqual(channel.logs, ["c", "de", "f"]);
});

test("Replaces", () => {
channel.appendLine("a");
channel.appendLine("b");
Expand All @@ -64,4 +74,11 @@ suite("SwiftOutputChannel", function () {
channel.replace("e");
assert.deepEqual(channel.logs, ["e"]);
});

test("AppendLine after append terminates appending line", () => {
channel.append("a");
channel.append("b");
channel.appendLine("c");
assert.deepEqual(channel.logs, ["ab", "c"]);
});
});