Fix crash when there are multiple packages to install per code fix action (#56400)

This commit is contained in:
Sheetal Nandi
2023-11-15 16:13:27 -08:00
committed by GitHub
parent 4515089840
commit 29bb857226
8 changed files with 1015 additions and 15 deletions

View File

@@ -51,6 +51,7 @@ export interface InstallPackageRequest extends TypingInstallerRequestWithProject
readonly fileName: Path;
readonly packageName: string;
readonly projectRootPath: Path;
readonly id: number;
}
/** @internal */
@@ -61,6 +62,7 @@ export interface TypesRegistryResponse extends TypingInstallerResponse {
export interface PackageInstalledResponse extends ProjectResponse {
readonly kind: ActionPackageInstalled;
readonly id: number;
readonly success: boolean;
readonly message: string;
}

View File

@@ -46,6 +46,11 @@ export interface TypingsInstallerWorkerProcess {
send<T extends TypingInstallerRequestUnion>(rq: T): void;
}
interface PackageInstallPromise {
resolve(value: ApplyCodeActionCommandResult): void;
reject(reason: unknown): void;
}
/** @internal */
export abstract class TypingsInstallerAdapter implements ITypingsInstaller {
protected installer!: TypingsInstallerWorkerProcess;
@@ -63,10 +68,8 @@ export abstract class TypingsInstallerAdapter implements ITypingsInstaller {
// It would be preferable to base our limit on the amount of space left in the
// buffer, but we have yet to find a way to retrieve that value.
private static readonly requestDelayMillis = 100;
private packageInstalledPromise: {
resolve(value: ApplyCodeActionCommandResult): void;
reject(reason: unknown): void;
} | undefined;
private packageInstalledPromise: Map<number, PackageInstallPromise> | undefined;
private packageInstallId = 0;
constructor(
protected readonly telemetryEnabled: boolean,
@@ -92,11 +95,13 @@ export abstract class TypingsInstallerAdapter implements ITypingsInstaller {
}
installPackage(options: InstallPackageOptionsWithProject): Promise<ApplyCodeActionCommandResult> {
this.installer.send<InstallPackageRequest>({ kind: "installPackage", ...options });
Debug.assert(this.packageInstalledPromise === undefined);
return new Promise<ApplyCodeActionCommandResult>((resolve, reject) => {
this.packageInstalledPromise = { resolve, reject };
this.packageInstallId++;
const request: InstallPackageRequest = { kind: "installPackage", ...options, id: this.packageInstallId };
const promise = new Promise<ApplyCodeActionCommandResult>((resolve, reject) => {
(this.packageInstalledPromise ??= new Map()).set(this.packageInstallId, { resolve, reject });
});
this.installer.send(request);
return promise;
}
attach(projectService: ProjectService) {
@@ -136,15 +141,15 @@ export abstract class TypingsInstallerAdapter implements ITypingsInstaller {
this.typesRegistryCache = new Map(Object.entries(response.typesRegistry));
break;
case ActionPackageInstalled: {
const { success, message } = response;
if (success) {
this.packageInstalledPromise!.resolve({ successMessage: message });
const promise = this.packageInstalledPromise?.get(response.id);
Debug.assertIsDefined(promise, "Should find the promise for package install");
this.packageInstalledPromise?.delete(response.id);
if (response.success) {
promise.resolve({ successMessage: response.message });
}
else {
this.packageInstalledPromise!.reject(message);
promise.reject(response.message);
}
this.packageInstalledPromise = undefined;
this.projectService.updateTypingsForProject(response);
// The behavior is the same as for setTypings, so send the same event.

View File

@@ -140,6 +140,7 @@ import "./unittests/tsserver/autoImportProvider";
import "./unittests/tsserver/auxiliaryProject";
import "./unittests/tsserver/cachingFileSystemInformation";
import "./unittests/tsserver/cancellationToken";
import "./unittests/tsserver/codeFix";
import "./unittests/tsserver/compileOnSave";
import "./unittests/tsserver/completions";
import "./unittests/tsserver/completionsIncomplete";

View File

@@ -0,0 +1,65 @@
import * as ts from "../../_namespaces/ts";
import {
dedent,
} from "../../_namespaces/Utils";
import {
baselineTsserverLogs,
openFilesForSession,
TestSession,
} from "../helpers/tsserver";
import {
createServerHost,
libFile,
} from "../helpers/virtualFileSystemWithWatch";
describe("unittests:: tsserver:: codeFix::", () => {
function setup() {
const host = createServerHost({
"/home/src/projects/project/src/file.ts": dedent`
import * as os from "os";
import * as https from "https";
import * as vscode from "vscode";
`,
"/home/src/projects/project/tsconfig.json": "{ }",
"/home/src/projects/project/node_modules/vscode/index.js": "export const x = 10;",
[libFile.path]: libFile.content,
});
const session = new TestSession({ host, typesRegistry: ["vscode"] });
openFilesForSession(["/home/src/projects/project/src/file.ts"], session);
const actions = session.executeCommandSeq<ts.server.protocol.GetCombinedCodeFixRequest>({
command: ts.server.protocol.CommandTypes.GetCombinedCodeFix,
arguments: {
scope: { type: "file", args: { file: "/home/src/projects/project/src/file.ts" } },
fixId: "installTypesPackage",
},
}).response as ts.server.protocol.CombinedCodeActions;
return { host, session, actions };
}
it("install package", () => {
const { host, session, actions } = setup();
actions.commands?.forEach(command =>
session.executeCommandSeq<ts.server.protocol.ApplyCodeActionCommandRequest>({
command: ts.server.protocol.CommandTypes.ApplyCodeActionCommand,
arguments: {
command,
},
})
);
host.runPendingInstalls();
baselineTsserverLogs("codeFix", "install package", session);
});
it("install package when serialized", () => {
const { host, session, actions } = setup();
actions.commands?.forEach(command => {
session.executeCommandSeq<ts.server.protocol.ApplyCodeActionCommandRequest>({
command: ts.server.protocol.CommandTypes.ApplyCodeActionCommand,
arguments: {
command,
},
});
host.runPendingInstalls();
});
baselineTsserverLogs("codeFix", "install package when serialized", session);
});
});

View File

@@ -233,7 +233,7 @@ export abstract class TypingsInstaller {
/** @internal */
installPackage(req: InstallPackageRequest) {
const { fileName, packageName, projectName, projectRootPath } = req;
const { fileName, packageName, projectName, projectRootPath, id } = req;
const cwd = forEachAncestorDirectory(getDirectoryPath(fileName), directory => {
if (this.installTypingHost.fileExists(combinePaths(directory, "package.json"))) {
return directory;
@@ -247,6 +247,7 @@ export abstract class TypingsInstaller {
const response: PackageInstalledResponse = {
kind: ActionPackageInstalled,
projectName,
id,
success,
message,
};
@@ -257,6 +258,7 @@ export abstract class TypingsInstaller {
const response: PackageInstalledResponse = {
kind: ActionPackageInstalled,
projectName,
id,
success: false,
message: "Could not determine a project root path.",
};