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,37 @@ 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/helpers/simpleGit.ts

@@ -33,6 +33,15 @@ 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();
         });