[fix] Isolate registry/package failures during sync instead of crashing

specification.md

@@ -304,7 +304,9 @@ The purpose of remote syncing is to call multiple registries and aggregate remot
 
 1. For each Registry in the configuration
 2. Call the API to load the list of package metadata
+   1. If the registry cannot be reached, or its response is invalid, record the failure and continue with the remaining registries rather than aborting the whole sync
 3. Combine packages from multiple registries into a single index
+   1. Run Package Validation (see [Scan logic](#scan-logic)) on each package version; if a version is invalid, record the failure and skip just that version, rather than aborting the rest of the sync
 4. Store package metadata in-memory as a read-only cache to speed up the app instead of making API requests constantly. Manager Local can store the aggregated registry on disk.
 
 #### Sync example

src/classes/Manager.ts

@@ -11,24 +11,31 @@ import { Architecture, SystemType } from '../index-browser.js';
 export class Manager extends Base {
   protected config: Config;
   protected packages: Map<string, Package>;
+  protected syncErrors: string[];
   type: RegistryType;
 
   constructor(type: RegistryType, config?: ConfigInterface) {
     super();
     this.config = new Config(config);
     this.packages = new Map();
+    this.syncErrors = [];
     this.type = type;
   }
 
   addPackage(pkg: Package) {
     let pkgExisting = this.packages.get(pkg.slug);
+    const isNewPackage: boolean = !pkgExisting;
     if (!pkgExisting) {
       pkgExisting = new Package(pkg.slug);
-      this.packages.set(pkg.slug, pkgExisting);
     }
     for (const [version, pkgVersion] of pkg.versions) {
+      // addVersion() throws on an invalid version - only register a brand-new package once its
+      // versions have been added successfully, so a caller that catches this (e.g. sync()
+      // isolating one bad version) doesn't end up with an orphaned, empty Package left behind
+      // in the index for a package that was never actually added.
       pkgExisting.addVersion(version, pkgVersion);
     }
+    if (isNewPackage) this.packages.set(pkg.slug, pkgExisting);
   }
 
   filter(method: (pkgVersion: PackageVersion, pkg: Package) => boolean): Package[] {
@@ -46,6 +53,10 @@ export class Manager extends Base {
     return this.packages.get(slug);
   }
 
+  getSyncErrors(): string[] {
+    return this.syncErrors;
+  }
+
   getReport() {
     const reports: ManagerReport = {};
     for (const [slug, pkg] of this.packages) {
@@ -96,6 +107,7 @@ export class Manager extends Base {
 
   reset() {
     this.packages.clear();
+    this.syncErrors = [];
   }
 
   search(query: string): Package[] {
@@ -118,13 +130,32 @@ export class Manager extends Base {
   }
 
   async sync() {
+    // Reset on each call - stale errors from a previous sync() shouldn't linger.
+    this.syncErrors = [];
     const registries: ConfigRegistry[] = this.config.get('registries') as ConfigRegistry[];
+    const type: RegistryType = this.type;
     for (const index in registries) {
-      const json: RegistryInterface = await apiJson(registries[index].url);
-      const type: RegistryType = this.type;
+      let json: RegistryInterface;
+      try {
+        json = await apiJson(registries[index].url);
+      } catch (err) {
+        // One unreachable/misconfigured registry shouldn't stop the others from being synced -
+        // record the failure and move on, matching the spec's goal of combining packages from
+        // multiple registries into a single index.
+        this.syncErrors.push(`${registries[index].name}: ${(err as Error).message}`);
+        continue;
+      }
       for (const slug in json[type]) {
-        const pkg = new Package(slug, json[type][slug].versions);
-        this.addPackage(pkg);
+        for (const version in json[type][slug].versions) {
+          try {
+            // Add one version at a time (rather than the whole package via addPackage() in one
+            // call) so a single malformed version - from a registry this manager doesn't
+            // control - can't abort every other version/package still left to sync.
+            this.addPackage(new Package(slug, { [version]: json[type][slug].versions[version] }));
+          } catch (err) {
+            this.syncErrors.push(`${slug}@${version}: ${(err as Error).message}`);
+          }
+        }
       }
     }
   }

tests/classes/Manager.test.ts

@@ -1,4 +1,4 @@
-import { expect, test } from 'vitest';
+import { expect, test, vi } from 'vitest';
 import {
   PLUGIN,
   PLUGIN_INCOMPATIBLE,
@@ -13,8 +13,10 @@ import { Package } from '../../src/classes/Package';
 import { License } from '../../src/types/License';
 import { SystemType } from '../../src/types/SystemType';
 import { Architecture } from '../../src/types/Architecture';
+import { PackageVersion } from '../../src/types/Package';
 import { packageCompatibleFiles } from '../../src/helpers/package';
 import { omitDownloads } from '../testUtils';
+import * as apiHelpers from '../../src/helpers/api';
 
 test('Manager add multiple package versions', () => {
   const manager = new Manager(RegistryType.Plugins);
@@ -191,6 +193,55 @@ test('Manager sync from registries', async () => {
   expect(omitDownloads(pkg?.getVersion(PLUGIN_PACKAGE.version))).toEqual(omitDownloads(PLUGIN));
 });
 
+test('Manager sync skips an unreachable registry instead of throwing', async () => {
+  // example.invalid is reserved by RFC 2606 specifically for cases like this - guaranteed to
+  // never resolve, so this doesn't depend on some third-party service happening to be down.
+  const manager = new Manager(RegistryType.Plugins, {
+    registries: [
+      { name: 'Unreachable Registry', url: 'https://example.invalid/registry' },
+      { name: 'Open Audio Registry', url: 'https://open-audio-stack.github.io/open-audio-stack-registry' },
+    ],
+  });
+  await expect(manager.sync()).resolves.not.toThrow();
+  expect(manager.getSyncErrors().length).toBeGreaterThan(0);
+
+  // The other, reachable registry should still have synced successfully.
+  const pkg = manager.getPackage(PLUGIN_PACKAGE.slug);
+  expect(omitDownloads(pkg?.getVersion(PLUGIN_PACKAGE.version))).toEqual(omitDownloads(PLUGIN));
+});
+
+test('Manager sync isolates a malformed package version instead of throwing', async () => {
+  const pluginInvalid: PackageVersion = structuredClone(PLUGIN);
+  delete (pluginInvalid as any).image;
+
+  const apiJsonSpy = vi.spyOn(apiHelpers, 'apiJson').mockResolvedValue({
+    name: 'Mock Registry',
+    url: 'https://example.invalid/mock',
+    version: '1.0.0',
+    [RegistryType.Plugins]: {
+      'test-org/good-plugin': { slug: 'test-org/good-plugin', version: '1.0.0', versions: { '1.0.0': PLUGIN } },
+      'test-org/bad-plugin': {
+        slug: 'test-org/bad-plugin',
+        version: '1.0.0',
+        versions: { '1.0.0': pluginInvalid },
+      },
+    },
+  });
+
+  const manager = new Manager(RegistryType.Plugins, {
+    registries: [{ name: 'Mock Registry', url: 'https://example.invalid/mock' }],
+  });
+  await expect(manager.sync()).resolves.not.toThrow();
+
+  expect(omitDownloads(manager.getPackage('test-org/good-plugin')?.getVersion('1.0.0'))).toEqual(omitDownloads(PLUGIN));
+  expect(manager.getPackage('test-org/bad-plugin')).toBeUndefined();
+  expect(manager.getSyncErrors()).toEqual(
+    expect.arrayContaining([expect.stringContaining('test-org/bad-plugin@1.0.0')]),
+  );
+
+  apiJsonSpy.mockRestore();
+});
+
 test('Manager sync with existing package', async () => {
   const manager = new Manager(RegistryType.Plugins);
   const pkg = new Package(PLUGIN_PACKAGE.slug);