From 67fb56b7150469bc615aeef2bdb683365a58431d Mon Sep 17 00:00:00 2001
From: Lukas Stracke <lukas.stracke@sentry.io>
Date: Mon, 4 Nov 2024 10:52:34 +0100
Subject: [PATCH] align error handling, strict flag, ValueError

---
 api-node/src/common/registry.service.ts      | 29 ++++++++------------
 api-node/src/common/utils.ts                 |  5 +++-
 api-node/src/common/valueError.ts            |  6 ++++
 api-node/src/packages/packages.controller.ts |  5 ++--
 api-node/src/sdks/sdks.controller.ts         |  4 +--
 5 files changed, 27 insertions(+), 22 deletions(-)
 create mode 100644 api-node/src/common/valueError.ts

diff --git a/api-node/src/common/registry.service.ts b/api-node/src/common/registry.service.ts
index 1e2300a28..4f631e2b3 100644
--- a/api-node/src/common/registry.service.ts
+++ b/api-node/src/common/registry.service.ts
@@ -11,6 +11,7 @@ import type { AppEntry, Apps } from '../apps/types';
 import type { PackageEntry, Packages } from '../packages/types';
 import type { AwsLambdaLayers } from '../aws-lambda-layers/types';
 import * as semver from 'semver';
+import { ValueError } from './valueError';
 
 const SDKS_PATH = path.join('..', 'sdks');
 const APPS_PATH = path.join('..', 'apps');
@@ -37,15 +38,15 @@ export class RegistryService {
           if (pkg) {
             sdks[link] = pkg;
           } else if (strict) {
-            throw new Error(
+            throw new ValueError(
               `Package ${link}, canonical cannot be resolved: ${canonical}`,
             );
           }
         } catch (error) {
-          if (strict) {
+          if (error instanceof ValueError) {
             throw error;
           }
-          // If not strict, continue to the next SDK
+          // IO and other errors are ignored
         }
       }
     } catch (error) {
@@ -73,22 +74,16 @@ export class RegistryService {
   // Packages
 
   getPackages(strict: boolean = false): Packages {
-    return Array.from(iterPackages()).reduce((acc, canonical) => {
-      const packageDir = getPackageDirFromCanonical(canonical);
-      const latestFilePath = path.join(packageDir, 'latest.json');
-
-      try {
-        const packageInfo = JSON.parse(
-          fs.readFileSync(latestFilePath).toString(),
+    return Array.from(iterPackages()).reduce((acc, pkgName) => {
+      const pkg = this.getPackage(pkgName);
+      if (!pkg && strict) {
+        throw new ValueError(
+          `Package does not exist or invalid canonical: ${pkgName}`,
         );
-        return {
-          ...acc,
-          [packageInfo.canonical]: packageInfo,
-        };
-      } catch (e) {
-        console.error(`Failed to read package: ${canonical}`);
-        console.error(e);
       }
+
+      acc[pkg.canonical] = pkg;
+      return acc;
     }, {});
   }
 
diff --git a/api-node/src/common/utils.ts b/api-node/src/common/utils.ts
index 05f4128ce..c0f59b5c3 100644
--- a/api-node/src/common/utils.ts
+++ b/api-node/src/common/utils.ts
@@ -1,3 +1,6 @@
-export function isTruthy(value: string): boolean {
+export function isTruthy(value: string | undefined): boolean {
+  if (!value) {
+    return false;
+  }
   return ['true', '1', 'yes'].includes(value.toLowerCase());
 }
diff --git a/api-node/src/common/valueError.ts b/api-node/src/common/valueError.ts
new file mode 100644
index 000000000..83b7f5205
--- /dev/null
+++ b/api-node/src/common/valueError.ts
@@ -0,0 +1,6 @@
+export class ValueError extends Error {
+  constructor(message: string) {
+    super(message);
+    this.name = 'ValueError';
+  }
+}
diff --git a/api-node/src/packages/packages.controller.ts b/api-node/src/packages/packages.controller.ts
index 0a94d025d..c9edbe96b 100644
--- a/api-node/src/packages/packages.controller.ts
+++ b/api-node/src/packages/packages.controller.ts
@@ -9,6 +9,7 @@ import {
 import { RegistryService } from '../common/registry.service';
 import { PackageEntry, Packages, PackageVersions } from './types';
 import { ReleaseRegistryCacheInterceptor } from '../common/cache';
+import { isTruthy } from 'src/common/utils';
 
 @Controller('packages')
 @UseInterceptors(ReleaseRegistryCacheInterceptor)
@@ -16,8 +17,8 @@ export class PackagesController {
   constructor(private registryService: RegistryService) {}
 
   @Get()
-  getPackages(@Query('strict') strict: boolean = false): Packages {
-    return this.registryService.getPackages(strict);
+  getPackages(@Query('strict') strict?: string): Packages {
+    return this.registryService.getPackages(isTruthy(strict));
   }
 
   @Get('/:package(*)/versions')
diff --git a/api-node/src/sdks/sdks.controller.ts b/api-node/src/sdks/sdks.controller.ts
index bc4b62842..da0387651 100644
--- a/api-node/src/sdks/sdks.controller.ts
+++ b/api-node/src/sdks/sdks.controller.ts
@@ -2,6 +2,7 @@ import { Controller, Get, Param, Query, UseInterceptors } from '@nestjs/common';
 import { SdkEntry, Sdks, SdkVersions } from './types';
 import { RegistryService } from '../common/registry.service';
 import { ReleaseRegistryCacheInterceptor } from '../common/cache';
+import { isTruthy } from 'src/common/utils';
 
 @Controller('sdks')
 @UseInterceptors(ReleaseRegistryCacheInterceptor)
@@ -10,8 +11,7 @@ export class SdksController {
 
   @Get()
   getSdks(@Query('strict') strict?: string): Sdks {
-    const isStrict =
-      strict?.toLowerCase() === 'true' || strict === '1' || strict === 'yes';
+    const isStrict = isTruthy(strict);
     return this.registryService.getSdks(isStrict);
   }