Skip to content

Commit

Permalink
load aspects error handling (#7286)
Browse files Browse the repository at this point in the history
  • Loading branch information
GiladShoham authored Apr 19, 2023
1 parent 652d0d3 commit a6e7ef8
Show file tree
Hide file tree
Showing 16 changed files with 186 additions and 46 deletions.
9 changes: 8 additions & 1 deletion e2e/harmony/load-extensions.e2e.4.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import stripAnsi from 'strip-ansi';
import chai, { expect } from 'chai';
import path from 'path';

Expand Down Expand Up @@ -100,7 +101,13 @@ describe('load extensions', function () {
});
it('should show a warning about the problematic extension', () => {
expect(output).to.have.string(
UNABLE_TO_LOAD_EXTENSION_FROM_LIST(['my-scope/extension-provider-error'], 'error in provider')
stripAnsi(
UNABLE_TO_LOAD_EXTENSION_FROM_LIST(
['my-scope/extension-provider-error'],
'error in provider',
'teambit.workspace/workspace (cli.registerOnStart)'
)
)
);
});
});
Expand Down
17 changes: 17 additions & 0 deletions scopes/component/component/component-factory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,23 @@ export type ResolveAspectsOptions = FilterAspectsOptions & {
};

export type LoadAspectsOptions = {
/* `throwOnError` is an optional parameter that can be passed to the loadAspects method in the `ComponentFactory` interface. If
set to `true`, it will cause the method to throw an error if an error occurs during its execution. If set to `false`
or not provided, the method will print a warning instead of throwing it. */
throwOnError?: boolean;
/* `hideMissingModuleError` is an optional parameter that can be passed to the `loadAspects` method in the
`ComponentFactory` interface. If set to `true`, it will prevent the method from throwing/printing an error if a required module
is missing during the loading of an aspect. Instead, it will continue loading the other
aspects. If set to `false` or not provided, the method will print/throw an error if a required module is missing.
(considering throwOnError as well) */
hideMissingModuleError?: boolean;

/* The `ignoreErrors` property is an optional boolean parameter that can be passed to the `LoadAspectsOptions` object in
the `ComponentFactory` interface. If set to `true`, it will cause the `loadAspects` method to ignore any errors that
occur during the loading of aspects and continue loading the other aspects. If set to `false` or not provided, the
method will print/throw an error if a required module is missing or if any other error occurs during the loading of
aspects. */
ignoreErrors?: boolean;
[key: string]: any;
};

Expand Down
7 changes: 6 additions & 1 deletion scopes/component/component/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,12 @@ export { Component, InvalidComponent } from './component';
export { ComponentID } from '@teambit/component-id';
export { default as ComponentFS } from './component-fs';
export type { default as ComponentConfig } from './config';
export type { ComponentFactory, ResolveAspectsOptions, FilterAspectsOptions } from './component-factory';
export type {
ComponentFactory,
ResolveAspectsOptions,
FilterAspectsOptions,
LoadAspectsOptions,
} from './component-factory';
export type { AspectList } from './aspect-list';
export { AspectEntry, AspectData, ResolveComponentIdFunc } from './aspect-entry';
// TODO: check why it's not working when using the index in snap dir like this:
Expand Down
2 changes: 1 addition & 1 deletion scopes/component/isolator/isolator.main.runtime.ts
Original file line number Diff line number Diff line change
Expand Up @@ -415,7 +415,7 @@ export class IsolatorMain {
longProcessLogger.end();
// this.logger.consoleSuccess();
const capsuleListOutput = allCapsuleList.map((capsule) => capsule.component.id.toString()).join(', ');
this.logger.consoleSuccess(`following capsule(s) ensured ${chalk.cyan(capsuleListOutput)}`);
this.logger.consoleSuccess(`resolved aspect(s): ${chalk.cyan(capsuleListOutput)}`);
}

return allCapsuleList;
Expand Down
4 changes: 2 additions & 2 deletions scopes/dependencies/pnpm/pnpm.package-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ export class PnpmPackageManager implements PackageManager {
if (!installOptions.hidePackageManagerOutput) {
// this.logger.setStatusLine('installing dependencies using pnpm');
// turn off the logger because it interrupts the pnpm output
this.logger.console('-------------------------PNPM OUTPUT-------------------------');
// this.logger.console('-------------------------PNPM OUTPUT-------------------------');
this.logger.off();
}
const registries = await this.depResolver.getRegistries();
Expand Down Expand Up @@ -82,7 +82,7 @@ export class PnpmPackageManager implements PackageManager {
if (!installOptions.hidePackageManagerOutput) {
this.logger.on();
// Make a divider row to improve output
this.logger.console('-------------------------END PNPM OUTPUT-------------------------');
// this.logger.console('-------------------------END PNPM OUTPUT-------------------------');
// this.logger.consoleSuccess('installing dependencies using pnpm');
}
}
Expand Down
20 changes: 19 additions & 1 deletion scopes/envs/envs/environments.main.runtime.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,8 @@ export type Descriptor = RegularCompDescriptor | EnvCompDescriptor;
export const DEFAULT_ENV = 'teambit.harmony/node';

export class EnvsMain {
private failedToLoadExt = new Set<string>();

static runtime = MainRuntime;

private alreadyShownWarning = {};
Expand Down Expand Up @@ -115,6 +117,22 @@ export class EnvsMain {
return this.createRuntime(components);
}

/**
*
* @param id
*/
/**
* This function adds an extension ID to a set of failed to load extensions.
* This mostly used by the aspect loader to add such issues
* Then it is used to hide different errors that are caused by the same issue.
* @param {string} id - string - represents the unique identifier of an extension that failed to load.
*/
addFailedToLoadExtension(id: string) {
if (!this.failedToLoadExt.has(id)) {
this.failedToLoadExt.add(id);
}
}

/**
* get the configured default env.
*/
Expand Down Expand Up @@ -617,7 +635,7 @@ export class EnvsMain {
}

private printWarningIfFirstTime(envId: string, message: string) {
if (!this.alreadyShownWarning[envId]) {
if (!this.alreadyShownWarning[envId] && !this.failedToLoadExt.has(envId)) {
this.alreadyShownWarning[envId] = true;
this.logger.consoleWarning(message);
}
Expand Down
62 changes: 56 additions & 6 deletions scopes/harmony/aspect-loader/aspect-loader.main.runtime.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import LegacyScope from '@teambit/legacy/dist/scope/scope';
import { GLOBAL_SCOPE, DEFAULT_DIST_DIRNAME } from '@teambit/legacy/dist/constants';
import { MainRuntime } from '@teambit/cli';
import { ExtensionManifest, Harmony, Aspect, SlotRegistry, Slot } from '@teambit/harmony';
import { BitError } from '@teambit/bit-error';
import type { LoggerMain } from '@teambit/logger';
import { ComponentID, Component, FilterAspectsOptions } from '@teambit/component';
import { Logger, LoggerAspect } from '@teambit/logger';
Expand All @@ -14,7 +15,7 @@ import { EnvsAspect, EnvsMain } from '@teambit/envs';
import { loadBit } from '@teambit/bit';
import { ScopeAspect, ScopeMain } from '@teambit/scope';
import mapSeries from 'p-map-series';
import { difference, compact, flatten, intersection, uniqBy } from 'lodash';
import { difference, compact, flatten, intersection, uniqBy, some } from 'lodash';
import { AspectDefinition, AspectDefinitionProps } from './aspect-definition';
import { PluginDefinition } from './plugin-definition';
import { AspectLoaderAspect } from './aspect-loader.aspect';
Expand Down Expand Up @@ -45,6 +46,22 @@ export type ResolvedAspect = {
aspectFilePath: string | null;
};

export type LoadExtByManifestContext = {
seeders?: string[];
neededFor?: string;
};

export type LoadExtByManifestOptions = {
throwOnError?: boolean;
hideMissingModuleError?: boolean;
ignoreErrors?: boolean;
/**
* If this is enabled then we will show loading error only once for a given extension
* (even if it was actually try to load it few times by different components for example)
*/
unifyErrorsByExtId?: boolean;
};

type OnAspectLoadError = (err: Error, id: ComponentID) => Promise<boolean>;
export type OnAspectLoadErrorSlot = SlotRegistry<OnAspectLoadError>;

Expand Down Expand Up @@ -93,6 +110,7 @@ export type MainAspect = {

export class AspectLoaderMain {
private inMemoryConfiguredAspects: string[] = [];
private failedToLoadExt = new Set<string>();

constructor(
private logger: Logger,
Expand Down Expand Up @@ -384,7 +402,7 @@ export class AspectLoaderMain {
*/
async loadRequireableExtensions(requireableExtensions: RequireableComponent[], throwOnError = false): Promise<void> {
const manifests = await this.getManifestsFromRequireableExtensions(requireableExtensions, throwOnError);
return this.loadExtensionsByManifests(manifests, throwOnError);
return this.loadExtensionsByManifests(manifests, undefined, { throwOnError });
}

async getManifestsFromRequireableExtensions(
Expand Down Expand Up @@ -574,9 +592,25 @@ export class AspectLoaderMain {

async loadExtensionsByManifests(
extensionsManifests: Array<ExtensionManifest | Aspect>,
throwOnError = true,
seeders: string[] = []
context: LoadExtByManifestContext = {
seeders: [],
},
options: LoadExtByManifestOptions = {
throwOnError: true,
hideMissingModuleError: false,
ignoreErrors: false,
unifyErrorsByExtId: true,
}
) {
const neededFor = context.neededFor;
const seeders = context.seeders || [];
const defaultLoadExtByManifestOptions = {
throwOnError: true,
hideMissingModuleError: false,
ignoreErrors: false,
unifyErrorsByExtId: true,
};
const mergedOptions = { ...defaultLoadExtByManifestOptions, ...options };
try {
const manifests = extensionsManifests.filter((manifest) => {
const isValid = this.isValidAspect(manifest);
Expand Down Expand Up @@ -604,15 +638,31 @@ export class AspectLoaderMain {
const ids = extensionsManifests.map((manifest) => manifest.id || 'unknown');
// TODO: improve texts
const errorMsg = e.message.split('\n')[0];
const warning = UNABLE_TO_LOAD_EXTENSION_FROM_LIST(ids, errorMsg);
const warning = UNABLE_TO_LOAD_EXTENSION_FROM_LIST(ids, errorMsg, neededFor);
this.logger.error(warning, e);
if (mergedOptions.ignoreErrors) return;
if (e.code === 'MODULE_NOT_FOUND' && mergedOptions.hideMissingModuleError) return;
if (mergedOptions.unifyErrorsByExtId) {
const needToPrint = some(ids, (id) => !this.failedToLoadExt.has(id));
if (!needToPrint) return;
ids.forEach((id) => {
this.failedToLoadExt.add(id);
this.envs.addFailedToLoadExtension(id);
const parsedId = ComponentID.tryFromString(id);
if (parsedId) {
this.failedToLoadExt.add(parsedId.fullName);
this.envs.addFailedToLoadExtension(parsedId.fullName);
}
});
}
if (this.logger.isLoaderStarted) {
if (mergedOptions.throwOnError) throw new BitError(warning);
this.logger.consoleFailure(warning);
} else {
this.logger.consoleWarning(warning);
this.logger.consoleWarning(e);
}
if (throwOnError) {
if (mergedOptions.throwOnError) {
throw e;
}
}
Expand Down
18 changes: 13 additions & 5 deletions scopes/harmony/aspect-loader/constants.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,16 @@
import chalk from 'chalk';

export const UNABLE_TO_LOAD_EXTENSION = (id: string, errMsg?: string) =>
`error: unable to load the extension "${id}", due to an error "${
errMsg || '<unknown-error>'
}", please use the '--log=error' flag for the full error.`;
export const UNABLE_TO_LOAD_EXTENSION_FROM_LIST = (ids: string[], errMsg?: string) =>
`couldn't load one of the following extensions ${ids.join(', ')}, due to an error "${
`error: Bit received an error loading "${id}", due to the error "${
errMsg || '<unknown-error>'
}", please use the '--log=error' flag for the full error.`;
export const UNABLE_TO_LOAD_EXTENSION_FROM_LIST = (ids: string[], errMsg?: string, neededFor?: string) => {
// const installOutput = err?.code === 'MODULE_NOT_FOUND' ? `try running "bit install" to install the missing dependencies` : '';
const installOutput = `try running ${chalk.cyan('"bit install"')} to fix this issue`;
return `Bit received an error loading ${chalk.cyan(ids.join(', '))}, due to the error:
"${errMsg || '<unknown-error>'}".
This is required for the component: ${chalk.cyan(neededFor || 'unknown')}
Please use the ${chalk.cyan("'--log=error'")} flag for the full error.
${installOutput}
`;
};
5 changes: 3 additions & 2 deletions scopes/harmony/cli/cli.main.runtime.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import { CliCmd, CliGenerateCmd } from './cli.cmd';
import { HelpCmd } from './help.cmd';

export type CommandList = Array<Command>;
export type OnStart = (hasWorkspace: boolean) => Promise<void>;
export type OnStart = (hasWorkspace: boolean, currentCommand: string) => Promise<void>;

export type OnStartSlot = SlotRegistry<OnStart>;
export type CommandsSlot = SlotRegistry<CommandList>;
Expand Down Expand Up @@ -100,7 +100,8 @@ export class CLIMain {

private async invokeOnStart(hasWorkspace: boolean) {
const onStartFns = this.onStartSlot.values();
await pMapSeries(onStartFns, (onStart) => onStart(hasWorkspace));
const currentCommand = process.argv[2];
await pMapSeries(onStartFns, (onStart) => onStart(hasWorkspace, currentCommand));
}

private setDefaults(command: Command) {
Expand Down
13 changes: 8 additions & 5 deletions scopes/scope/scope/scope-aspects-loader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,16 +10,17 @@ import { compact, uniq, difference, groupBy } from 'lodash';
import { MainRuntime } from '@teambit/cli';
import { RequireableComponent } from '@teambit/harmony.modules.requireable-component';
import { ExtensionManifest, Aspect } from '@teambit/harmony';
import { Component, ComponentID, ResolveAspectsOptions } from '@teambit/component';
import { Component, ComponentID, LoadAspectsOptions, ResolveAspectsOptions } from '@teambit/component';
import { ScopeMain } from '@teambit/scope';
import { Logger } from '@teambit/logger';
import { EnvsMain } from '@teambit/envs';

type ManifestOrAspect = ExtensionManifest | Aspect;

export type ScopeLoadAspectsOptions = {
export type ScopeLoadAspectsOptions = LoadAspectsOptions & {
useScopeAspectsCapsule?: boolean;
packageManagerConfigRootDir?: string;
workspaceName?: string;
};

export class ScopeAspectsLoader {
Expand Down Expand Up @@ -64,7 +65,7 @@ export class ScopeAspectsLoader {
return module.default || module;
});

await this.aspectLoader.loadExtensionsByManifests(manifests, true);
await this.aspectLoader.loadExtensionsByManifests(manifests, undefined, { throwOnError: true });
}

private localAspects: string[] = [];
Expand Down Expand Up @@ -145,6 +146,7 @@ needed-for: ${neededFor || '<unknown>'}`);
lane?: Lane,
opts: {
packageManagerConfigRootDir?: string;
workspaceName?: string;
} = {}
): Promise<{ manifests: ManifestOrAspect[]; potentialPluginsIds: string[] }> {
ids = uniq(ids);
Expand Down Expand Up @@ -229,7 +231,7 @@ needed-for: ${neededFor || '<unknown>'}`);

async getResolvedAspects(
components: Component[],
opts?: { skipIfExists?: boolean; packageManagerConfigRootDir?: string }
opts?: { skipIfExists?: boolean; packageManagerConfigRootDir?: string; workspaceName?: string }
): Promise<RequireableComponent[]> {
if (!components || !components.length) return [];
const network = await this.isolator.isolateComponents(
Expand All @@ -249,6 +251,7 @@ needed-for: ${neededFor || '<unknown>'}`);
},
context: {
aspects: true,
workspaceName: opts?.workspaceName,
},
},
this.scope.legacyScope
Expand Down Expand Up @@ -324,7 +327,7 @@ needed-for: ${neededFor || '<unknown>'}`);
async requireAspects(
components: Component[],
throwOnError = false,
opts: { packageManagerConfigRootDir?: string } = {}
opts: { packageManagerConfigRootDir?: string; workspaceName?: string } = {}
): Promise<Array<ExtensionManifest | Aspect>> {
const requireableExtensions = await this.getResolvedAspects(components, opts);
if (!requireableExtensions) {
Expand Down
3 changes: 1 addition & 2 deletions scopes/workspace/install/install.cmd.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,7 @@ export default class InstallCmd implements Command {
const executionTime = calculateTime(startTime, endTime);
return `Successfully installed dependencies and compiled ${chalk.cyan(
components.toArray().length.toString()
)} component(s) in ${chalk.cyan(executionTime.toString())} seconds
Your workspace is ready to use 😃`;
)} component(s) in ${chalk.cyan(executionTime.toString())} seconds`;
}
}

Expand Down
6 changes: 5 additions & 1 deletion scopes/workspace/install/install.main.runtime.ts
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,8 @@ export class InstallMain {
* @memberof Workspace
*/
async install(packages?: string[], options?: WorkspaceInstallOptions): Promise<ComponentMap<string>> {
// set workspace in install context
this.workspace.inInstallContext = true;
if (packages && packages.length) {
await this._addPackages(packages, options);
}
Expand Down Expand Up @@ -136,7 +138,9 @@ export class InstallMain {
}
}
await pMapSeries(this.preInstallSlot.values(), (fn) => fn(options)); // import objects if not disabled in options
return this._installModules(options);
const res = await this._installModules(options);
this.workspace.inInstallContext = false;
return res;
}

registerPreLink(fn: PreLink) {
Expand Down
5 changes: 3 additions & 2 deletions scopes/workspace/workspace/aspects-merger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { partition, mergeWith, merge } from 'lodash';
import { MergeConfigConflict } from './exceptions/merge-config-conflict';
import { AspectSpecificField, ExtensionsOrigin, Workspace } from './workspace';
import { MergeConflictFile } from './merge-conflict-file';
import { WorkspaceLoadAspectsOptions } from './workspace-aspects-loader';

export class AspectsMerger {
private consumer: Consumer;
Expand Down Expand Up @@ -319,10 +320,10 @@ export class AspectsMerger {
private async loadExtensions(
extensions: ExtensionDataList,
originatedFrom?: ComponentID,
throwOnError = false
opts: WorkspaceLoadAspectsOptions = {}
): Promise<void> {
const workspaceAspectsLoader = this.workspace.getWorkspaceAspectsLoader();
return workspaceAspectsLoader.loadComponentsExtensions(extensions, originatedFrom, throwOnError);
return workspaceAspectsLoader.loadComponentsExtensions(extensions, originatedFrom, opts);
}

/**
Expand Down
Loading

0 comments on commit a6e7ef8

Please sign in to comment.