Merge pull request #164 from launchcodedev/swallowing-not-found-errors

Avoid swallowing transitive NotFoundError

Author: joelgallant

app-config-config/src/index.ts

@@ -82,7 +82,7 @@ export async function loadUnvalidatedConfig({
     return { parsed, fullConfig: parsed.toJSON() };
   } catch (error) {
     // having no APP_CONFIG environment variable is normal, and should fall through to reading files
-    if (!(error instanceof NotFoundError)) throw error;
+    if (!NotFoundError.isNotFoundError(error)) throw error;
   }
 
   const meta = await loadMetaConfig({ directory });
@@ -123,7 +123,7 @@ export async function loadUnvalidatedConfig({
       .read(secretsFileExtensions)
       .catch((error) => {
         // NOTE: secrets are optional, so not finding them is normal
-        if (error instanceof NotFoundError) {
+        if (NotFoundError.isNotFoundError(error, join(directory, secretsFileNameBase))) {
           logger.verbose('Did not find secrets file');
           return undefined;
         }
@@ -160,7 +160,7 @@ export async function loadUnvalidatedConfig({
       parsed = ParsedValue.merge(parsed, parsedExtension);
     } catch (error) {
       // having no APP_CONFIG_CI environment variable is normal, and should fall through to reading files
-      if (!(error instanceof NotFoundError)) throw error;
+      if (!NotFoundError.isNotFoundError(error)) throw error;
     }
   }
 

app-config-core/src/config-source.ts

@@ -5,7 +5,13 @@ import { parse as parseJSON5, stringify as stringifyJSON5 } from 'json5';
 import { Json, JsonObject } from '@app-config/utils';
 import { logger } from '@app-config/logging';
 import { ParsedValue, ParsingContext, ParsingExtension } from './parsed-value';
-import { AppConfigError, NotFoundError, ParsingError, BadFileType } from './errors';
+import {
+  AppConfigError,
+  NotFoundError,
+  ParsingError,
+  BadFileType,
+  FallbackExhaustedError,
+} from './errors';
 
 /**
  * File formats that app-config supports.
@@ -22,6 +28,8 @@ export enum FileType {
 
 /** Base class for "sources", which are strategies to read configuration (eg. files, environment variables) */
 export abstract class ConfigSource {
+  public readonly filePath?: string;
+
   /** Only method that is *required* for all ConfigSources, which is built on in readValue, read, and readToJSON */
   protected abstract readContents(): Promise<[string, FileType]>;
 
@@ -122,6 +130,8 @@ export class FallbackSource extends ConfigSource {
 
   // override because readContents uses it (which is backwards from super class)
   async readValue(): Promise<Json> {
+    const errors: NotFoundError[] = [];
+
     // take the first value that comes back without an error
     for (const source of this.sources) {
       try {
@@ -130,19 +140,28 @@ export class FallbackSource extends ConfigSource {
 
         return value;
       } catch (error) {
-        if (error instanceof NotFoundError) {
+        if (source.filePath) {
+          // special case for ConfigSource with `filePath`, only accept a NotFoundError for it's filePath
+          if (NotFoundError.isNotFoundError(error, source.filePath)) {
+            errors.push(error);
+            continue;
+          }
+        } else if (NotFoundError.isNotFoundError(error)) {
+          errors.push(error);
           continue;
         }
 
         throw error;
       }
     }
 
-    throw new NotFoundError('FallbackSource found no valid ConfigSource');
+    throw new FallbackExhaustedError('FallbackSource found no valid ConfigSource', errors);
   }
 
   // override so that ParsedValue is directly from the originating ConfigSource
   async read(extensions?: ParsingExtension[], context?: ParsingContext): Promise<ParsedValue> {
+    const errors: NotFoundError[] = [];
+
     // take the first value that comes back without an error
     for (const source of this.sources) {
       try {
@@ -151,15 +170,22 @@ export class FallbackSource extends ConfigSource {
 
         return value;
       } catch (error) {
-        if (error instanceof NotFoundError) {
+        if (source.filePath) {
+          // special case for ConfigSource with `filePath`, only accept a NotFoundError for it's filePath
+          if (NotFoundError.isNotFoundError(error, source.filePath)) {
+            errors.push(error);
+            continue;
+          }
+        } else if (NotFoundError.isNotFoundError(error)) {
+          errors.push(error);
           continue;
         }
 
         throw error;
       }
     }
 
-    throw new NotFoundError('FallbackSource found no valid ConfigSource');
+    throw new FallbackExhaustedError('FallbackSource found no valid ConfigSource', errors);
   }
 }
 

app-config-core/src/errors.ts

@@ -5,7 +5,35 @@ export class AppConfigError extends Error {}
 export class Fallbackable extends AppConfigError {}
 
 /** When a ConfigSource cannot be found */
-export class NotFoundError extends Fallbackable {}
+export class NotFoundError extends Fallbackable {
+  constructor(message?: string, public readonly filepath?: string) {
+    super(message);
+  }
+
+  static isNotFoundError(err: Error | unknown, filepath?: string): err is NotFoundError {
+    if (err instanceof NotFoundError) {
+      if (filepath) {
+        return err.filepath === filepath;
+      }
+
+      return true;
+    }
+
+    return false;
+  }
+}
+
+export class EnvironmentVariableNotFoundError extends NotFoundError {
+  constructor(message?: string, public readonly environmentVariable?: string) {
+    super(message);
+  }
+}
+
+export class FallbackExhaustedError extends NotFoundError {
+  constructor(message: string, public readonly errors: NotFoundError[]) {
+    super(message);
+  }
+}
 
 /** Could not parse a string from a config source */
 export class ParsingError extends AppConfigError {}

app-config-extensions/src/extends-directive.ts

@@ -57,7 +57,7 @@ function fileReferenceDirective(keyName: string, meta: ParsedValueMetadata): Par
               environmentOptions,
             })
             .catch((error) => {
-              if (error instanceof NotFoundError && isOptional) {
+              if (isOptional && NotFoundError.isNotFoundError(error, resolvedPath)) {
                 return ParsedValue.literal({});
               }
 

app-config-meta/src/index.test.ts

@@ -160,4 +160,19 @@ describe('meta file loading', () => {
       },
     );
   });
+
+  it('bubbles up and does not ignore a NotFoundError from transitively included files', async () => {
+    await withTempFiles(
+      {
+        '.app-config.meta.yml': `
+          $extends: a-missing-file.yml
+        `,
+      },
+      async (inDir) => {
+        await expect(loadMetaConfig({ directory: inDir('.') })).rejects.toThrow(
+          `File ${inDir('a-missing-file.yml')} not found`,
+        );
+      },
+    );
+  });
 });

app-config-meta/src/index.ts

@@ -6,8 +6,8 @@ import {
   ParsingExtension,
   FallbackSource,
   FileType,
-  NotFoundError,
   AppConfigError,
+  FallbackExhaustedError,
 } from '@app-config/core';
 import { logger } from '@app-config/logging';
 
@@ -87,9 +87,9 @@ export async function loadMetaConfig({
   }
 
   // we try to find meta find in our CWD, but fallback to the workspace (git repo) root
-  const sources = [new FlexibleFileSource(join(directory, fileNameBase))];
+  const sources = [new FlexibleFileSource(join(resolve(directory), fileNameBase))];
 
-  if (workspaceRoot) {
+  if (workspaceRoot && workspaceRoot !== directory) {
     sources.push(new FlexibleFileSource(join(workspaceRoot, fileNameBase)));
   }
 
@@ -110,12 +110,18 @@ export async function loadMetaConfig({
       value,
     };
   } catch (error) {
-    if (error instanceof NotFoundError) {
-      logger.verbose(
-        `Meta file was not found in ${directory} or workspace root (${workspaceRoot ?? 'none'})`,
-      );
-
-      return { value: {} };
+    if (error instanceof FallbackExhaustedError) {
+      for (const { filepath } of error.errors) {
+        if (sources.some((s) => s.filePath === filepath)) {
+          logger.verbose(
+            `Meta file was not found in ${directory} or workspace root (${
+              workspaceRoot ?? 'none'
+            })`,
+          );
+
+          return { value: {} };
+        }
+      }
     }
 
     throw error;

app-config-node/src/environment-source.ts

@@ -1,4 +1,9 @@
-import { guessFileType, ConfigSource, FileType, NotFoundError } from '@app-config/core';
+import {
+  guessFileType,
+  ConfigSource,
+  FileType,
+  EnvironmentVariableNotFoundError,
+} from '@app-config/core';
 import { logger } from '@app-config/logging';
 
 /** Read configuration from an environment variable */
@@ -11,7 +16,10 @@ export class EnvironmentSource extends ConfigSource {
     const value = process.env[this.variableName];
 
     if (!value) {
-      throw new NotFoundError(`Could not read the environment variable '${this.variableName}'`);
+      throw new EnvironmentVariableNotFoundError(
+        `Could not read the environment variable '${this.variableName}'`,
+        this.variableName,
+      );
     }
 
     const inferredFileType = await guessFileType(value);

app-config-node/src/file-source.ts

@@ -40,7 +40,7 @@ export class FileSource extends ConfigSource {
       return [content.toString('utf-8'), this.fileType];
     } catch (err: unknown) {
       if (err && typeof err === 'object' && (err as { code?: string | number }).code === 'ENOENT') {
-        throw new NotFoundError(`File ${this.filePath} not found`);
+        throw new NotFoundError(`File ${this.filePath} not found`, this.filePath);
       }
 
       throw err;
@@ -50,7 +50,7 @@ export class FileSource extends ConfigSource {
 
 /** Read configuration from a file, found via "glob-like" search (any file format, with support for environment specific files) */
 export class FlexibleFileSource extends ConfigSource {
-  private readonly filePath: string;
+  public readonly filePath: string;
   private readonly fileExtensions: string[];
   private readonly environmentOptions: EnvironmentOptions;
 
@@ -138,6 +138,7 @@ export class FlexibleFileSource extends ConfigSource {
 
     throw new NotFoundError(
       `FlexibleFileSource could not find file with ${this.filePath}.{yml|yaml|toml|json|json5}`,
+      this.filePath,
     );
   }
 

app-config-schema/src/index.ts

@@ -62,7 +62,7 @@ export async function loadSchema({
 
   parsed = await env.read(parsingExtensions).catch((error) => {
     // having no APP_CONFIG_SCHEMA environment variable is normal, and should fall through to reading files
-    if (error instanceof NotFoundError) {
+    if (NotFoundError.isNotFoundError(error)) {
       return undefined;
     }
 

app-config-settings/src/index.ts

@@ -45,7 +45,9 @@ export async function loadSettings(): Promise<Settings> {
 
     return value;
   } catch (error) {
-    if (error instanceof NotFoundError) return {};
+    if (NotFoundError.isNotFoundError(error, path)) {
+      return {};
+    }
 
     throw error;
   }