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: report errors when analyzing extensions #4380

Merged
merged 4 commits into from Nov 14, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions packages/main/src/plugin/extension-loader.spec.ts
Expand Up @@ -673,6 +673,7 @@ describe('analyze extension and main', async () => {
const extension = await extensionLoader.analyzeExtension(path.resolve('/', 'fake', 'path'), false);

expect(extension).toBeDefined();
expect(extension?.error).toBeDefined();
expect(extension?.mainPath).toBe(path.resolve('/', 'fake', 'path', 'main-entry.js'));
expect(extension?.id).toBe('fooPublisher.fooName');
});
Expand All @@ -697,6 +698,7 @@ describe('analyze extension and main', async () => {
const extension = await extensionLoader.analyzeExtension('/fake/path', false);

expect(extension).toBeDefined();
expect(extension?.error).toBeDefined();
// not set
expect(extension?.mainPath).toBeUndefined();
expect(extension?.id).toBe('fooPublisher.fooName');
Expand Down
43 changes: 33 additions & 10 deletions packages/main/src/plugin/extension-loader.ts
Expand Up @@ -93,6 +93,8 @@ export interface AnalyzedExtension {
missingDependencies?: string[];
circularDependencies?: string[];

error?: string;

readonly subscriptions: { dispose(): unknown }[];

dispose(): void;
Expand Down Expand Up @@ -202,7 +204,7 @@ export class ExtensionLoader {
zipper.sync.unzip(filePath).save(unpackedDirectory);

const extension = await this.analyzeExtension(unpackedDirectory, true);
if (extension) {
if (!extension.error) {
await this.loadExtension(extension);
this.apiSender.send('extension-started', {});
}
Expand Down Expand Up @@ -293,12 +295,12 @@ export class ExtensionLoader {

const analyzedFoldersExtension = (
await Promise.all(folders.map(folder => this.analyzeExtension(folder, false)))
).filter(extension => extension !== undefined) as AnalyzedExtension[];
).filter(extension => !extension.error) as AnalyzedExtension[];
deboer-tim marked this conversation as resolved.
Show resolved Hide resolved
analyzedExtensions.push(...analyzedFoldersExtension);

const analyzedExternalExtensions = (
await Promise.all(externalExtensions.map(folder => this.analyzeExtension(folder, false)))
).filter(extension => extension !== undefined) as AnalyzedExtension[];
).filter(extension => !extension.error) as AnalyzedExtension[];
deboer-tim marked this conversation as resolved.
Show resolved Hide resolved
analyzedExtensions.push(...analyzedExternalExtensions);

// also load extensions from the plugins directory
Expand All @@ -312,27 +314,47 @@ export class ExtensionLoader {
// collect all extensions from the pluginDirectory folders
const analyzedPluginsDirectoryExtensions: AnalyzedExtension[] = (
await Promise.all(pluginDirectories.map(folder => this.analyzeExtension(folder, true)))
).filter(extension => extension !== undefined) as AnalyzedExtension[];
).filter(extension => !extension.error) as AnalyzedExtension[];
deboer-tim marked this conversation as resolved.
Show resolved Hide resolved
analyzedExtensions.push(...analyzedPluginsDirectoryExtensions);
}

// now we have all extensions, we can load them
await this.loadExtensions(analyzedExtensions);
}

async analyzeExtension(extensionPath: string, removable: boolean): Promise<AnalyzedExtension | undefined> {
async analyzeExtension(extensionPath: string, removable: boolean): Promise<AnalyzedExtension> {
// do nothing if there is no package.json file
let error = undefined;
const disposables: Disposable[] = [];
if (!fs.existsSync(path.resolve(extensionPath, 'package.json'))) {
console.warn(`Ignoring extension ${extensionPath} without package.json file`);
return undefined;
error = `Ignoring extension ${extensionPath} without package.json file`;
console.warn(error);
const analyzedExtension: AnalyzedExtension = {
id: '<unknown>',
name: '<unknown>',
path: extensionPath,
manifest: undefined,
api: <typeof containerDesktopAPI>{},
removable,
subscriptions: disposables,
dispose(): void {
disposables.forEach(disposable => disposable.dispose());
},
deboer-tim marked this conversation as resolved.
Show resolved Hide resolved
error,
};
return analyzedExtension;
}

// log error if the manifest is missing required entries
const manifest = await this.loadManifest(extensionPath);
if (!manifest.name || !manifest.displayName || !manifest.version || !manifest.publisher || !manifest.description) {
error = `Extension ${extensionPath} missing required manifest entry in package.json (name, displayName, version, publisher, description)`;
console.warn(error);
}

// create api object
const api = this.createApi(extensionPath, manifest);

const disposables: Disposable[] = [];
const analyzedExtension: AnalyzedExtension = {
id: `${manifest.publisher}.${manifest.name}`,
name: manifest.name,
Expand All @@ -345,6 +367,7 @@ export class ExtensionLoader {
dispose(): void {
disposables.forEach(disposable => disposable.dispose());
},
error,
};

return analyzedExtension;
Expand Down Expand Up @@ -508,7 +531,7 @@ export class ExtensionLoader {
try {
const updatedExtension = await this.analyzeExtension(extension.path, removable);

if (updatedExtension) {
if (!updatedExtension.error) {
await this.loadExtension(updatedExtension, true);
}
} catch (error) {
Expand Down Expand Up @@ -1227,7 +1250,7 @@ export class ExtensionLoader {
if (extension) {
const analyzedExtension = await this.analyzeExtension(extension.path, extension.removable);

if (analyzedExtension) {
if (!analyzedExtension.error) {
await this.loadExtension(analyzedExtension, true);
}
}
Expand Down
5 changes: 5 additions & 0 deletions packages/main/src/plugin/install/extension-installer.ts
Expand Up @@ -166,6 +166,11 @@ export class ExtensionInstaller {
} catch (error) {
sendError('Error while analyzing extension: ' + error);
}
if (analyzedExtension?.error) {
sendError('Could not load extension: ' + analyzedExtension?.error);
return;
}

return analyzedExtension;
}

Expand Down