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.

Sign up for GitHub

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

[plugin] handle plugin host activation exceptions #8103

Merged
merged 1 commit into from
Jun 29, 2020
Merged

[plugin] handle plugin host activation exceptions #8103

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
64 changes: 30 additions & 34 deletions packages/plugin-ext/src/hosted/node/plugin-host-rpc.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,47 +102,43 @@ export class PluginHostRPC {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
loadPlugin(plugin: Plugin): any {
console.log('PLUGIN_HOST(' + process.pid + '): PluginManagerExtImpl/loadPlugin(' + plugin.pluginPath + ')');
try {
// cleaning the cache for all files of that plug-in.
Object.keys(require.cache).forEach(function (key): void {
const mod: NodeJS.Module = require.cache[key];
// cleaning the cache for all files of that plug-in.
Object.keys(require.cache).forEach(function (key): void {
const mod: NodeJS.Module = require.cache[key];

// attempting to reload a native module will throw an error, so skip them
if (mod.id.endsWith('.node')) {
return;
}
// attempting to reload a native module will throw an error, so skip them
if (mod.id.endsWith('.node')) {
return;
}

// remove children that are part of the plug-in
let i = mod.children.length;
while (i--) {
const childMod: NodeJS.Module = mod.children[i];
// ensure the child module is not null, is in the plug-in folder, and is not a native module (see above)
if (childMod && childMod.id.startsWith(plugin.pluginFolder) && !childMod.id.endsWith('.node')) {
// cleanup exports - note that some modules (e.g. ansi-styles) define their
// exports in an immutable manner, so overwriting the exports throws an error
delete childMod.exports;
mod.children.splice(i, 1);
for (let j = 0; j < childMod.children.length; j++) {
delete childMod.children[j];
}
// remove children that are part of the plug-in
let i = mod.children.length;
while (i--) {
const childMod: NodeJS.Module = mod.children[i];
// ensure the child module is not null, is in the plug-in folder, and is not a native module (see above)
if (childMod && childMod.id.startsWith(plugin.pluginFolder) && !childMod.id.endsWith('.node')) {
// cleanup exports - note that some modules (e.g. ansi-styles) define their
// exports in an immutable manner, so overwriting the exports throws an error
delete childMod.exports;
mod.children.splice(i, 1);
for (let j = 0; j < childMod.children.length; j++) {
delete childMod.children[j];
}
}
}

if (key.startsWith(plugin.pluginFolder)) {
// delete entry
delete require.cache[key];
const ix = mod.parent!.children.indexOf(mod);
if (ix >= 0) {
mod.parent!.children.splice(ix, 1);
}
if (key.startsWith(plugin.pluginFolder)) {
// delete entry
delete require.cache[key];
const ix = mod.parent!.children.indexOf(mod);
if (ix >= 0) {
mod.parent!.children.splice(ix, 1);
}

});
if (plugin.pluginPath) {
return require(plugin.pluginPath);
}
} catch (e) {
console.error(e);

});
if (plugin.pluginPath) {
return require(plugin.pluginPath);
}
},
async init(raw: PluginMetadata[]): Promise<[Plugin[], Plugin[]]> {
Expand Down
46 changes: 19 additions & 27 deletions packages/plugin-ext/src/plugin/plugin-manager.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -269,28 +269,30 @@ export class PluginManagerExtImpl implements PluginManagerExt, PluginManager {
if (plugin.rawModel.extensionDependencies) {
for (const dependencyId of plugin.rawModel.extensionDependencies) {
const dependency = this.registry.get(dependencyId.toLowerCase());
const id = plugin.model.displayName || plugin.model.id;
if (dependency) {
const depId = dependency.model.displayName || dependency.model.id;
const loadedSuccessfully = await this.loadPlugin(dependency, configStorage, visited);
if (!loadedSuccessfully) {
const message = `Cannot activate extension '${id}' because it depends on extension '${depId}', which failed to activate.`;
this.messageRegistryProxy.$showMessage(MainMessageType.Error, message, {}, []);
return false;
throw new Error(`Dependent extension '${dependency.model.displayName || dependency.model.id}' failed to activate.`);
}
} else {
const message = `Cannot activate the '${id}' extension because it depends on the '${dependencyId}' extension, which is not installed.`;
this.messageRegistryProxy.$showMessage(MainMessageType.Error, message, {}, []);
console.warn(message);
return false;
throw new Error(`Dependent extension '${dependencyId}' is not installed.`);
}
}
}

let pluginMain = this.host.loadPlugin(plugin);
// see https://github.com/TypeFox/vscode/blob/70b8db24a37fafc77247de7f7cb5bb0195120ed0/src/vs/workbench/api/common/extHostExtensionService.ts#L372-L376
pluginMain = pluginMain || {};
return await this.startPlugin(plugin, configStorage, pluginMain);
await this.startPlugin(plugin, configStorage, pluginMain);
return true;
} catch (err) {
if (this.pluginActivationPromises.has(plugin.model.id)) {
thegecko marked this conversation as resolved.
Show resolved Hide resolved
this.pluginActivationPromises.get(plugin.model.id)!.reject(err);
}
const message = `Activating extension '${plugin.model.displayName || plugin.model.name}' failed: ${err.message}`;
this.messageRegistryProxy.$showMessage(MainMessageType.Error, message, {}, []);
Copy link
Member

@akosyakov akosyakov Jun 29, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was wondering maybe instead of using message service, we should have a special service then you can custoimize it in your product to completely stop the execution if some core extensions are failed, by default it will delegate to the notification center.

Copy link
Member Author

@thegecko thegecko Jun 29, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this idea, but it feels like a separate PR to change architecture rather than this bug fix.
I'll consider as the approach for our situation if that's OK?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

console.error(message);
return false;
} finally {
this.notificationMain.$stopProgress(progressId);
}
Expand Down Expand Up @@ -330,7 +332,7 @@ export class PluginManagerExtImpl implements PluginManagerExt, PluginManager {
}

// eslint-disable-next-line @typescript-eslint/no-explicit-any
private async startPlugin(plugin: Plugin, configStorage: ConfigStorage, pluginMain: any): Promise<boolean> {
private async startPlugin(plugin: Plugin, configStorage: ConfigStorage, pluginMain: any): Promise<void> {
const subscriptions: theia.Disposable[] = [];
const asAbsolutePath = (relativePath: string): string => join(plugin.pluginFolder, relativePath);
const logPath = join(configStorage.hostLogPath, plugin.model.id); // todo check format
Expand All @@ -354,29 +356,19 @@ export class PluginManagerExtImpl implements PluginManagerExt, PluginManager {
}
const id = plugin.model.displayName || plugin.model.id;
if (typeof pluginMain[plugin.lifecycle.startMethod] === 'function') {
try {
const pluginExport = await pluginMain[plugin.lifecycle.startMethod].apply(getGlobal(), [pluginContext]);
this.activatedPlugins.set(plugin.model.id, new ActivatedPlugin(pluginContext, pluginExport, stopFn));
const pluginExport = await pluginMain[plugin.lifecycle.startMethod].apply(getGlobal(), [pluginContext]);
this.activatedPlugins.set(plugin.model.id, new ActivatedPlugin(pluginContext, pluginExport, stopFn));

// resolve activation promise
if (this.pluginActivationPromises.has(plugin.model.id)) {
this.pluginActivationPromises.get(plugin.model.id)!.resolve();
this.pluginActivationPromises.delete(plugin.model.id);
}
} catch (err) {
if (this.pluginActivationPromises.has(plugin.model.id)) {
this.pluginActivationPromises.get(plugin.model.id)!.reject(err);
}
this.messageRegistryProxy.$showMessage(MainMessageType.Error, `Activating extension ${id} failed: ${err.message}.`, {}, []);
console.error(`Error on activation of ${plugin.model.name}`, err);
return false;
// resolve activation promise
if (this.pluginActivationPromises.has(plugin.model.id)) {
this.pluginActivationPromises.get(plugin.model.id)!.resolve();
this.pluginActivationPromises.delete(plugin.model.id);
}
} else {
// https://github.com/TypeFox/vscode/blob/70b8db24a37fafc77247de7f7cb5bb0195120ed0/src/vs/workbench/api/common/extHostExtensionService.ts#L400-L401
console.log(`plugin ${id}, ${plugin.lifecycle.startMethod} method is undefined so the module is the extension's exports`);
this.activatedPlugins.set(plugin.model.id, new ActivatedPlugin(pluginContext, pluginMain));
}
return true;
}

getAllPlugins(): Plugin[] {
Expand Down