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

load aspects error handling #7286

Merged
merged 14 commits into from
Apr 19, 2023
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