Improve error handling

This intends to give better error handling for velocitas init and velocitas sync

src/modules/package-downloader.ts

@@ -13,7 +13,7 @@
 // SPDX-License-Identifier: Apache-2.0
 
 import { join } from 'node:path';
-import { CheckRepoActions, SimpleGit, simpleGit } from 'simple-git';
+import { CheckRepoActions, ResetMode, SimpleGit, simpleGit } from 'simple-git';
 import { CliFileSystem } from '../utils/fs-bridge';
 import { PackageConfig } from './package';
 import { BRANCH_PREFIX } from './semver';
@@ -26,8 +26,11 @@ export class PackageDownloader {
  this.packageConfig = packageConfig;
  }
 
- private async _cloneRepository(packageDir: string, cloneOpts: string[]): Promise<void> {
- await this.git.clone(this.packageConfig.getPackageRepo(), packageDir, cloneOpts);
+ private async _cloneRepository(packageDir: string, cloneOpts: string[], verbose?: boolean): Promise<void> {
+ const response = await this.git.clone(this.packageConfig.getPackageRepo(), packageDir, cloneOpts);
+ if (verbose) {
+ console.log(`Clone response: ${response}`);
+ }
  }
 
  private async _updateRepository(checkRepoAction: CheckRepoActions, verbose: boolean): Promise<void> {
@@ -46,18 +49,40 @@ export class PackageDownloader {
 
  private async _checkoutVersion(version: string): Promise<void> {
  const branchOrTag = version.startsWith(BRANCH_PREFIX) ? `origin/${version.substring(BRANCH_PREFIX.length)}` : version;
+ // Make sure that repo is clean to avoid that checkout fails!
+ await this.git.reset(ResetMode.HARD);
  await this.git.checkout(branchOrTag);
  }
 
- private async _checkForValidRepo(packageDir: string, cloneOpts: string[], checkRepoAction: CheckRepoActions): Promise<void> {
+ private async _checkForValidRepo(
+ packageDir: string,
+ cloneOpts: string[],
+ checkRepoAction: CheckRepoActions,
+ verbose?: boolean,
+ ): Promise<void> {
  let directoryExists = CliFileSystem.existsSync(packageDir);
  if (directoryExists && !(await this.isValidRepo(packageDir, checkRepoAction))) {
  CliFileSystem.removeSync(packageDir);
  directoryExists = false;
  }
 
  if (!directoryExists) {
- await this._cloneRepository(packageDir, cloneOpts);
+ try {
+ // simple-git typically throws an error if clone fails
+ // but not all errors reult in an exception, for instance blocked clone due to rate limitation
+ // does not result in an exception
+ await this._cloneRepository(packageDir, cloneOpts, verbose);
+ } catch (error) {
+ if (verbose) {
+ console.error(error);
+ }
+ throw new Error(`Cloning of ${this.packageConfig.getPackageRepo()} failed!`);
+ }
+
+ // Do a second check to verify if clone seems to have succeeded
+ if (!(await this.isValidRepo(packageDir, checkRepoAction))) {
+ throw new Error(`Problem detected when cloning ${this.packageConfig.getPackageRepo()}!`);
+ }
  }
  }
 
@@ -80,7 +105,7 @@ export class PackageDownloader {
  checkRepoAction = CheckRepoActions.IS_REPO_ROOT;
  }
 
- await this._checkForValidRepo(packageDir, cloneOpts, checkRepoAction);
+ await this._checkForValidRepo(packageDir, cloneOpts, checkRepoAction, verbose);
  this.git = simpleGit(packageDir);
  await this._updateRepository(checkRepoAction, verbose);
 

src/modules/package.ts

@@ -93,29 +93,33 @@ export class PackageConfig {
  }
 
  async getPackageVersions(verbose?: boolean): Promise<TagResult> {
- try {
-  const packageInformation = await packageDownloader(this).downloadPackage({ checkVersionOnly: true, verbose: verbose });
-  const packageVersionTags = await packageInformation.tags();
-  return packageVersionTags;
+ const packageInformation = await packageDownloader(this).downloadPackage({ checkVersionOnly: true, verbose: verbose });
+ const packageVersionTags = await packageInformation.tags();
+ return packageVersionTags;
+ /*
  } catch (error) {
+ console.log(`vvv`);
  console.log(error);
+ console.log(`www`);
  }
  return {
  all: [],
  latest: '',
- };
+ };*/
  }
 
  async downloadPackageVersion(verbose?: boolean): Promise<void> {
  try {
  await packageDownloader(this).downloadPackage({ verbose: verbose });
  } catch (error) {
- console.error(error);
+ if (verbose) {
+ console.error(error);
+ }
  // If repo exist but not version we will end up with default-version
  // of that repo, and on subsequent runs tooling will say that the missing version
  // actually exists! To prevent this we remove the directory of the missing version!
  CliFileSystem.removeSync(this.getPackageDirectoryWithVersion());
- throw new Error(`Cannot find package ${this.getPackageName()}:${this.version}`);
+ throw new Error(`Downloading package ${this.getPackageName()}:${this.version} failed!`);
  }
  return;
  }
@@ -125,7 +129,20 @@ export class PackageConfig {
  }
 
  async isPackageValidRepo(verbose?: boolean): Promise<boolean> {
- const isValid = await packageDownloader(this).isValidRepo(this.getPackageDirectoryWithVersion());
+ // When this is called we have likely already done _resolveVersion so this first check is not likely to fail
+ let isValid = await packageDownloader(this).isValidRepo(this.getPackageDirectoryWithVersion());
+ if (isValid) {
+ // Lets do a sanity check that there actually is a manifest file
+ const path = this.getManifestFilePath();
+ try {
+ let data = CliFileSystem.readFileSync(path);
+ } catch (error) {
+ if (verbose) {
+ console.log(`Cannot find manifest file ${path}`);
+ }
+ isValid = false;
+ }
+ }
  if (!isValid && verbose) {
  console.log(`... Corrupted .git directory found for: '${this.getPackageName()}:${this.version}'`);
  }
@@ -138,8 +155,10 @@ export class PackageConfig {
  const path = this.getManifestFilePath();
  data = CliFileSystem.readFileSync(path);
  } catch (error) {
- console.log(`Cannot find package ${this.getPackageName()}:${this.version}. Please upgrade or init first!`);
- throw new Error(`Cannot find package ${this.getPackageName()}:${this.version}`);
+ console.log(
+ `Cannot find package ${this.getPackageName()}:${this.version}. Please verify that it is a valid Velocitas package and do upgrade/init!`,
+ );
+ throw new Error(`Cannot find package ${this.getPackageName()}:${this.version} at ${this.getManifestFilePath()}`);
  }
  try {
  const config: PackageManifest = deserializePackageJSON(data);

src/modules/projectConfig/projectConfigFileReader.ts

@@ -46,18 +46,21 @@ export class MultiFormatConfigReader implements IProjectConfigReader {
  let config: ProjectConfig | null = null;
 
  for (const reader of projectConfigReaders) {
+ let first: boolean = true;
  try {
  config = reader.read(cliVersion, path, ignoreLock);
  if (config !== null) {
  break;
  }
  } catch (error: any) {
- console.warn(`Warning: ${path} not in expected format: ${error.message}, falling back to legacy format reading.`);
+ // This could be format error, but it could also be that repo/tag-settings are faulty, repo cannot be reached and
+ // similar, for example that "velocitas init" did not succeed!
+ console.warn(`Error reported by config reader ${reader.constructor.name}: ${error.message}`);
  }
  }
 
  if (config === null) {
- throw new Error(`Unable to read ${path}: unknown format!`);
+ throw new Error(`Unable to successfully read and interpret ${path}!`);
  }
 
  return config;

src/modules/projectConfig/projectConfigLock.ts

@@ -32,7 +32,7 @@ export class ProjectConfigLock implements ProjectConfigLockAttributes {
  public findVersion(packageName: string): string {
  const packageVersion = this.packages.get(packageName);
  if (!packageVersion) {
- throw new Error(`Package '${packageName}' not found in lock file.`);
+ throw new Error(`Package '${packageName}' not found in lock file. Have you performed "velocitas init"?`);
  }
  return packageVersion;
  }

test/commands/init/init.test.ts

@@ -92,21 +92,69 @@ describe('init', () => {
  });
 
  test.do(() => {
+ // Make sure that one package lack manifest.json
  mockFolders({ velocitasConfig: true, packageIndex: true, installedComponents: true });
+ CliFileSystem.removeSync(
+ `${userHomeDir}/.velocitas/packages/${runtimePackageInfoMock.repo}/${runtimePackageInfoMock.resolvedVersion}/manifest.json`,
+ );
+ CliFileSystem.promisesMkdir(
+ `${userHomeDir}/.velocitas/packages/${runtimePackageInfoMock.repo}/${runtimePackageInfoMock.resolvedVersion}`,
+ );
  })
  .stdout()
- .stub(gitModule, 'simpleGit', (stub) => stub.returns(simpleGitInstanceMock(undefined, false)))
+ .stub(gitModule, 'simpleGit', (stub) => stub.returns(simpleGitInstanceMock(undefined, true)))
  .stub(exec, 'runExecSpec', (stub) => stub.returns({}))
  .command(['init', '-v'])
- .it('downloads corrupted packages again', (ctx) => {
+ .it('refreshing corrupted package', (ctx) => {
  expect(ctx.stdout).to.contain('Initializing Velocitas packages ...');
+
  expect(ctx.stdout).to.contain(
- `... Corrupted .git directory found for: '${corePackageInfoMock.repo}:${corePackageInfoMock.resolvedVersion}'`,
+ `... Corrupted .git directory found for: '${runtimePackageInfoMock.repo}:${runtimePackageInfoMock.resolvedVersion}'`,
+ `Some error ${ctx.stdout}`,
+ );
+ expect(ctx.stdout).to.contain(
+ `... Downloading package: '${runtimePackageInfoMock.repo}:${runtimePackageInfoMock.resolvedVersion}'`,
  );
- expect(ctx.stdout).to.contain(`... Downloading package: '${corePackageInfoMock.repo}:${corePackageInfoMock.resolvedVersion}'`);
  expect(
  CliFileSystem.existsSync(
- `${userHomeDir}/.velocitas/packages/${corePackageInfoMock.repo}/${corePackageInfoMock.resolvedVersion}`,
+ `${userHomeDir}/.velocitas/packages/${runtimePackageInfoMock.repo}/${runtimePackageInfoMock.resolvedVersion}`,
+ ),
+ );
+ expect(
+ CliFileSystem.existsSync(
+ `${userHomeDir}/.velocitas/packages/${runtimePackageInfoMock.repo}/${runtimePackageInfoMock.resolvedVersion}/manifest.json`,
+ ),
+ ).to.be.true;
+ });
+
+ test.do(() => {
+ // Make sure that one package lack manifest.json
+ mockFolders({ velocitasConfig: true, packageIndex: true, installedComponents: true });
+ CliFileSystem.removeSync(
+ `${userHomeDir}/.velocitas/packages/${runtimePackageInfoMock.repo}/${runtimePackageInfoMock.resolvedVersion}/.git`,
+ );
+ CliFileSystem.promisesMkdir(
+ `${userHomeDir}/.velocitas/packages/${runtimePackageInfoMock.repo}/${runtimePackageInfoMock.resolvedVersion}`,
+ );
+ })
+ .stdout()
+ .stub(gitModule, 'simpleGit', (stub) => stub.returns(simpleGitInstanceMock(undefined, true)))
+ .stub(exec, 'runExecSpec', (stub) => stub.returns({}))
+ .command(['init', '-v'])
+ .it('refreshing folder that is not git folder', (ctx) => {
+ expect(ctx.stdout).to.contain('Initializing Velocitas packages ...');
+
+ expect(ctx.stdout).to.contain(
+ `... Downloading package: '${runtimePackageInfoMock.repo}:${runtimePackageInfoMock.resolvedVersion}'`,
+ );
+ expect(
+ CliFileSystem.existsSync(
+ `${userHomeDir}/.velocitas/packages/${runtimePackageInfoMock.repo}/${runtimePackageInfoMock.resolvedVersion}`,
+ ),
+ );
+ expect(
+ CliFileSystem.existsSync(
+ `${userHomeDir}/.velocitas/packages/${runtimePackageInfoMock.repo}/${runtimePackageInfoMock.resolvedVersion}/manifest.json`,
  ),
  ).to.be.true;
  });

test/helpers/simpleGit.ts

@@ -13,7 +13,8 @@
 // SPDX-License-Identifier: Apache-2.0
 
 import { CliFileSystem } from '../../src/utils/fs-bridge';
-import { corePackageManifestMock, runtimePackageManifestMock, setupPackageManifestMock } from '../utils/mockConfig';
+import { corePackageManifestMock, runtimePackageInfoMock, runtimePackageManifestMock, setupPackageManifestMock } from '../utils/mockConfig';
+import { userHomeDir } from '../utils/mockfs';
 
 export const simpleGitInstanceMock = (mockedNewVersionTag?: string, checkRepo: boolean = true) => {
  return {
@@ -33,6 +34,14 @@ export const simpleGitInstanceMock = (mockedNewVersionTag?: string, checkRepo: b
  return checkRepo;
  },
  fetch: () => {},
+ reset: async () => {
+ // This is to recover in test case "refreshing corrupted package"
+ // In the case we have "corrupted" the repo by removing the manifest
+ await CliFileSystem.promisesWriteFile(
+ `${userHomeDir}/.velocitas/packages/${runtimePackageInfoMock.repo}/${runtimePackageInfoMock.resolvedVersion}/manifest.json`,
+ JSON.stringify(runtimePackageManifestMock),
+ );
+ },
  checkout: () => {
  // Function implementation
  },

test/unit/projectConfigIO.test.ts

@@ -221,7 +221,7 @@ describe('projectConfigIO - module', () => {
  it('should handle errors when parsing .velocitas.json file', () => {
  const readFileStub = sinon.stub(CliFileSystem, 'readFileSync').throws();
  const projectConfigFileReader = () => ProjectConfigIO.read('', configFilePath, true);
- expect(projectConfigFileReader).to.throw(`Unable to read ${configFilePath}: unknown format!`);
+ expect(projectConfigFileReader).to.throw(`Unable to successfully read and interpret ${configFilePath}`);
  readFileStub.restore();
  });