Skip to content
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

Fix: stop start build server when getRandomPipepath return empty #1545

Merged
merged 7 commits into from
Aug 2, 2024
Merged
Show file tree
Hide file tree
Changes from 3 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
3 changes: 2 additions & 1 deletion .vscode/launch.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@
"order": 1
},
"env": {
"DEBUG_VSCODE_JAVA":"true"
"DEBUG_VSCODE_JAVA":"true",
"DEBUG_START_BUILD_SERVER": "true"
Jiaaming marked this conversation as resolved.
Show resolved Hide resolved
},
},
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
import java.util.concurrent.CompletionException;

import org.eclipse.core.internal.resources.Project;
import org.eclipse.core.internal.resources.ProjectDescription;
Expand All @@ -31,6 +32,8 @@
import org.eclipse.jdt.ls.core.internal.managers.BasicFileDetector;
import org.eclipse.jdt.ls.core.internal.managers.DigestStore;
import org.eclipse.jdt.ls.core.internal.preferences.Preferences;
import org.eclipse.lsp4j.jsonrpc.ResponseErrorException;

import com.microsoft.java.builder.JavaProblemChecker;
import com.microsoft.gradle.bs.importer.model.BuildServerPreferences;
import com.microsoft.gradle.bs.importer.model.Telemetry;
Expand All @@ -54,6 +57,7 @@ public class GradleBuildServerProjectImporter extends AbstractProjectImporter {
public static final String SETTINGS_GRADLE_DESCRIPTOR = "settings.gradle";
public static final String SETTINGS_GRADLE_KTS_DESCRIPTOR = "settings.gradle.kts";
public static final String ANDROID_MANIFEST = "AndroidManifest.xml";
private boolean IS_RESOLVED = true;
Jiaaming marked this conversation as resolved.
Show resolved Hide resolved

@Override
public boolean applies(IProgressMonitor monitor) throws OperationCanceledException, CoreException {
Expand Down Expand Up @@ -172,9 +176,21 @@ public void importToWorkspace(IProgressMonitor monitor) throws OperationCanceled
);
BuildServerPreferences data = getBuildServerPreferences();
params.setData(data);
InitializeBuildResult initializeResult = buildServer.buildInitialize(params).join();
buildServer.onBuildInitialized();
// TODO: save the capabilities of this server
try {
InitializeBuildResult initializeResult = buildServer.buildInitialize(params).join();
buildServer.onBuildInitialized();
// TODO: save the capabilities of this server
} catch (CompletionException e) {
Throwable cause = e.getCause();
if (cause instanceof ResponseErrorException) {
ResponseErrorException responseError = (ResponseErrorException) cause;
Jiaaming marked this conversation as resolved.
Show resolved Hide resolved
if ("Unhandled method build/initialize".equals(responseError.getMessage())) {
JavaLanguageServerPlugin.logException("Failed to start Gradle Build Server, use BuildShip instead", null);
this.IS_RESOLVED = false;
return;
}
}
}

if (monitor.isCanceled()) {
return;
Expand Down Expand Up @@ -208,7 +224,7 @@ public boolean isResolved(File folder) throws OperationCanceledException, CoreEx
// the gradle project has already imported by other importers, we can modify this logic
// so that Maven importer can be involved for other projects.
for (IProject project : ProjectUtils.getAllProjects()) {
if (Utils.isGradleBuildServerProject(project) &&
if (this.IS_RESOLVED && Utils.isGradleBuildServerProject(project) &&
project.getLocation().toPath().startsWith(folder.toPath())) {
return true;
}
Expand Down
1 change: 0 additions & 1 deletion extension/src/Extension.ts
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,6 @@ export class Extension {
});
}
const activated = !!(await this.rootProjectsStore.getProjectRoots()).length;
this.bspProxy.prepareToStart();
if (!this.server.isReady()) {
await this.server.start();
}
Expand Down
43 changes: 28 additions & 15 deletions extension/src/bs/BspProxy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import { sendInfo } from "vscode-extension-telemetry-wrapper";
export class BspProxy {
private buildServerConnector: BuildServerConnector;
private jdtlsImporterConnector: JdtlsImporterConnector;
private buildServerStart: boolean;

constructor(context: vscode.ExtensionContext, private readonly logger: Logger) {
this.buildServerConnector = new BuildServerConnector();
Expand All @@ -24,8 +25,8 @@ export class BspProxy {
/**
* This function needs to be called before we start Java Gradle Server.
*/
public prepareToStart(): void {
this.buildServerConnector.setupBuildServerPipeStream();
public prepareToStart(): boolean {
return this.buildServerConnector.setupBuildServerPipeStream();
}

/**
Expand All @@ -37,11 +38,12 @@ export class BspProxy {
public async start(): Promise<void> {
await this.jdtlsImporterConnector.waitForImporterPipePath();
await this.jdtlsImporterConnector.setupImporterPipeStream();

this.setupMessageForwarding(
this.jdtlsImporterConnector.getImporterConnection(),
this.buildServerConnector.getServerConnection()
);
if (this.buildServerStart) {
this.setupMessageForwarding(
this.jdtlsImporterConnector.getImporterConnection(),
this.buildServerConnector.getServerConnection()
);
}
this.jdtlsImporterConnector.startListening();
}

Expand All @@ -53,20 +55,28 @@ export class BspProxy {
importerConnection: rpc.MessageConnection | null,
buildServerConnection: rpc.MessageConnection | null
): void {
importerConnection?.onRequest((method, params) => {
if (!importerConnection) {
Jiaaming marked this conversation as resolved.
Show resolved Hide resolved
this.logger.error("Failed to setup importer message forwarding");
return;
}
if (!buildServerConnection) {
this.logger.error("Failed to setup build server message forwarding");
return;
}
importerConnection.onRequest((method, params) => {
if (params !== null) {
return buildServerConnection?.sendRequest(method, params);
return buildServerConnection.sendRequest(method, params);
}
return buildServerConnection?.sendRequest(method);
return buildServerConnection.sendRequest(method);
});

buildServerConnection?.onNotification((method, params) => {
buildServerConnection.onNotification((method, params) => {
if (params !== null) {
return importerConnection?.sendNotification(method, params);
return importerConnection.sendNotification(method, params);
}
importerConnection?.sendNotification(method);
importerConnection.sendNotification(method);
});
importerConnection?.onError(([error]) => {
importerConnection.onError(([error]) => {
this.logger.error(`Error on importerConnection: ${error.message}`);
sendInfo("", {
kind: "bspProxy-importerConnectionError",
Expand All @@ -76,7 +86,7 @@ export class BspProxy {
// TODO: Implement more specific error handling logic here
});

buildServerConnection?.onError(([error]) => {
buildServerConnection.onError(([error]) => {
this.logger.error(`Error on buildServerConnection: ${error.message}`);
sendInfo("", {
kind: "bspProxy-buildServerConnectionError",
Expand All @@ -86,6 +96,9 @@ export class BspProxy {
// TODO: Implement more specific error handling logic here
});
}
public setBuildServerStarted(started: boolean): void {
this.buildServerStart = started;
}

public closeConnection(): void {
this.buildServerConnector.close();
Expand Down
6 changes: 5 additions & 1 deletion extension/src/bs/BuildServerConnector.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,11 @@ export class BuildServerConnector {
* Generates a random pipe name, creates a pipe server and
* waiting for the connection from the Java build server.
*/
public setupBuildServerPipeStream(): void {
public setupBuildServerPipeStream(): boolean {
this.serverPipePath = getRandomPipeName();
if (this.serverPipePath === "") {
return false;
}
this.serverPipeServer = net.createServer((socket: net.Socket) => {
this.serverConnection = rpc.createMessageConnection(
new rpc.StreamMessageReader(socket),
Expand All @@ -25,6 +28,7 @@ export class BuildServerConnector {
this.serverConnection.listen();
});
this.serverPipeServer.listen(this.serverPipePath);
return true;
}

public getServerConnection(): rpc.MessageConnection | null {
Expand Down
14 changes: 12 additions & 2 deletions extension/src/server/GradleServer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,17 @@ export class GradleServer {
return this.languageServerPipePath;
}
public async start(): Promise<void> {
const isPrepared = this.bspProxy.prepareToStart();
if (!isPrepared) {
this.logger.error("Failed to generate build server pipe path, build server will not start");
}
let startBuildServer = isPrepared && redHatJavaInstalled();
if (process.env.DEBUG_VSCODE_JAVA) {
const debugBuildServer = process.env.DEBUG_START_BUILD_SERVER === "true";
startBuildServer = startBuildServer && debugBuildServer;
}
this.bspProxy.setBuildServerStarted(startBuildServer);

this.taskServerPort = await getPort();
const cwd = this.context.asAbsolutePath("lib");
const cmd = path.join(cwd, getGradleServerCommand());
Expand All @@ -61,13 +72,12 @@ export class GradleServer {
await vscode.window.showErrorMessage(NO_JAVA_EXECUTABLE);
return;
}
const startBuildServer = redHatJavaInstalled() ? "true" : "false";
const args = [
quoteArg(`--port=${this.taskServerPort}`),
quoteArg(`--startBuildServer=${startBuildServer}`),
quoteArg(`--languageServerPipePath=${this.languageServerPipePath}`),
];
if (startBuildServer === "true") {
if (startBuildServer) {
const buildServerPipeName = this.bspProxy.getBuildServerPipeName();
args.push(quoteArg(`--pipeName=${buildServerPipeName}`));
args.push(quoteArg(`--bundleDir=${bundleDirectory}`));
Expand Down