fix: remove FF edge_functions_invalid_config_throw (netlify/edge-bund…

…ler#374) Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
netlify · May 5, 2023 · 58694b2 · 58694b2
1 parent 7a5d0c7
commit 58694b2
Show file tree

Hide file tree

Showing 5 changed files with 37 additions and 94 deletions.
diff --git a/packages/edge-bundler/node/bundler.ts b/packages/edge-bundler/node/bundler.ts
@@ -113,11 +113,11 @@ const bundle = async (
   // Retrieving a configuration object for each function.
   // Run `getFunctionConfig` in parallel as it is a non-trivial operation and spins up deno
   const internalConfigPromises = internalFunctions.map(
-    async (func) => [func.name, await getFunctionConfig(func, importMap, deno, logger, featureFlags)] as const,
+    async (func) => [func.name, await getFunctionConfig(func, importMap, deno, logger)] as const,
   )
 
   const userConfigPromises = userFunctions.map(
-    async (func) => [func.name, await getFunctionConfig(func, importMap, deno, logger, featureFlags)] as const,
+    async (func) => [func.name, await getFunctionConfig(func, importMap, deno, logger)] as const,
   )
 
   // Creating a hash of function names to configuration objects.

diff --git a/packages/edge-bundler/node/config.test.ts b/packages/edge-bundler/node/config.test.ts
@@ -10,7 +10,7 @@ import { fixturesDir, useFixture } from '../test/util.js'
 
 import { DenoBridge } from './bridge.js'
 import { bundle } from './bundler.js'
-import { getFunctionConfig } from './config.js'
+import { FunctionConfig, getFunctionConfig } from './config.js'
 import type { Declaration } from './declaration.js'
 import { ImportMap } from './import_map.js'
 
@@ -24,7 +24,16 @@ const importMapFile = {
 const invalidDefaultExportErr = (path: string) =>
   `Default export in '${path}' must be a function. More on the Edge Functions API at https://ntl.fyi/edge-api.`
 
-const functions = [
+interface TestFunctions {
+  error?: RegExp
+  expectedConfig?: FunctionConfig
+  name: string
+  source: string
+  testName: string
+  userLog?: string
+}
+
+const functions: TestFunctions[] = [
   {
     testName: 'no config',
     expectedConfig: {},
@@ -42,8 +51,7 @@ const functions = [
       `,
   },
   {
-    testName: 'config with wrong type (throw)',
-    expectedConfig: {},
+    testName: 'config with wrong type',
     name: 'func3',
     source: `
       export default async () => new Response("Hello from function two")
@@ -52,27 +60,9 @@ const functions = [
     `,
     error:
       /^The 'config' export in edge function at '(.*)' must be an object\. More on the Edge Functions API at https:\/\/ntl\.fyi\/edge-api\.$/,
-    featureFlags: {
-      edge_functions_invalid_config_throw: true,
-    },
-  },
-  {
-    testName: 'config with wrong type (log)',
-    expectedConfig: {},
-    name: 'func3',
-    source: `
-      export default async () => new Response("Hello from function two")
-
-      export const config = () => ({})
-    `,
-    userLog: /^'config' export in edge function at '(.*)' must be an object$/,
-    featureFlags: {
-      edge_functions_invalid_config_throw: false,
-    },
   },
   {
-    testName: 'config with syntax error (throw)',
-    expectedConfig: {},
+    testName: 'config with syntax error',
     name: 'func4',
     source: `
       export default async () => new Response("Hello from function two")
@@ -81,23 +71,6 @@ const functions = [
     `,
     error:
       /^Could not load edge function at '(.*)'\. More on the Edge Functions API at https:\/\/ntl\.fyi\/edge-api\.$/,
-    featureFlags: {
-      edge_functions_invalid_config_throw: true,
-    },
-  },
-  {
-    testName: 'config with syntax error (log)',
-    expectedConfig: {},
-    name: 'func4',
-    source: `
-      export default async () => new Response("Hello from function two")
-
-      export const config
-    `,
-    userLog: /^Could not load edge function at '(.*)'$/,
-    featureFlags: {
-      edge_functions_invalid_config_throw: false,
-    },
   },
   {
     testName: 'config with correct onError',
@@ -171,7 +144,6 @@ describe('`getFunctionConfig` extracts configuration properties from function fi
         new ImportMap([importMapFile]),
         deno,
         logger,
-        func.featureFlags || {},
       )
 
     if (func.error) {
@@ -270,7 +242,6 @@ test('Passes validation if default export exists and is a function', async () =>
       new ImportMap([importMapFile]),
       deno,
       logger,
-      {},
     ),
   ).resolves.not.toThrow()
 
@@ -307,7 +278,6 @@ test('Fails validation if default export is not function', async () => {
     new ImportMap([importMapFile]),
     deno,
     logger,
-    {},
   )
 
   await expect(config).rejects.toThrowError(invalidDefaultExportErr(path))
@@ -344,7 +314,6 @@ test('Fails validation if default export is not present', async () => {
     new ImportMap([importMapFile]),
     deno,
     logger,
-    {},
   )
 
   await expect(config).rejects.toThrowError(invalidDefaultExportErr(path))

diff --git a/packages/edge-bundler/node/config.ts b/packages/edge-bundler/node/config.ts
@@ -7,7 +7,6 @@ import tmp from 'tmp-promise'
 import { DenoBridge } from './bridge.js'
 import { BundleError } from './bundle_error.js'
 import { EdgeFunction } from './edge_function.js'
-import { FeatureFlags } from './feature_flags.js'
 import { ImportMap } from './import_map.js'
 import { Logger } from './logger.js'
 import { getPackagePath } from './package_json.js'
@@ -53,14 +52,7 @@ const getConfigExtractor = () => {
   return configExtractorPath
 }
 
-export const getFunctionConfig = async (
-  func: EdgeFunction,
-  importMap: ImportMap,
-  deno: DenoBridge,
-  log: Logger,
-  featureFlags: FeatureFlags,
-  // eslint-disable-next-line max-params
-) => {
+export const getFunctionConfig = async (func: EdgeFunction, importMap: ImportMap, deno: DenoBridge, log: Logger) => {
   // The extractor is a Deno script that will import the function and run its
   // `config` export, if one exists.
   const extractorPath = getConfigExtractor()
@@ -93,7 +85,7 @@ export const getFunctionConfig = async (
   )
 
   if (exitCode !== ConfigExitCode.Success) {
-    handleConfigError(func, exitCode, stderr, log, featureFlags)
+    handleConfigError(func, exitCode, stderr, log)
 
     return {}
   }
@@ -108,7 +100,7 @@ export const getFunctionConfig = async (
     const collectorDataJSON = await fs.readFile(collector.path, 'utf8')
     collectorData = JSON.parse(collectorDataJSON) as FunctionConfig
   } catch {
-    handleConfigError(func, ConfigExitCode.UnhandledError, stderr, log, featureFlags)
+    handleConfigError(func, ConfigExitCode.UnhandledError, stderr, log)
   } finally {
     await collector.cleanup()
   }
@@ -124,26 +116,16 @@ export const getFunctionConfig = async (
   return collectorData
 }
 
-const handleConfigError = (
-  func: EdgeFunction,
-  exitCode: number,
-  stderr: string,
-  log: Logger,
-  featureFlags: FeatureFlags,
-  // eslint-disable-next-line max-params
-) => {
+const handleConfigError = (func: EdgeFunction, exitCode: number, stderr: string, log: Logger) => {
   switch (exitCode) {
     case ConfigExitCode.ImportError:
       log.user(stderr)
-      if (featureFlags.edge_functions_invalid_config_throw) {
-        throw new BundleError(
-          new Error(
-            `Could not load edge function at '${func.path}'. More on the Edge Functions API at https://ntl.fyi/edge-api.`,
-          ),
-        )
-      } else {
-        log.user(`Could not load edge function at '${func.path}'`)
-      }
+      throw new BundleError(
+        new Error(
+          `Could not load edge function at '${func.path}'. More on the Edge Functions API at https://ntl.fyi/edge-api.`,
+        ),
+      )
+
       break
 
     case ConfigExitCode.NoConfig:
@@ -152,27 +134,20 @@ const handleConfigError = (
       break
 
     case ConfigExitCode.InvalidExport:
-      if (featureFlags.edge_functions_invalid_config_throw) {
-        throw new BundleError(
-          new Error(
-            `The 'config' export in edge function at '${func.path}' must be an object. More on the Edge Functions API at https://ntl.fyi/edge-api.`,
-          ),
-        )
-      } else {
-        log.user(`'config' export in edge function at '${func.path}' must be an object`)
-      }
+      throw new BundleError(
+        new Error(
+          `The 'config' export in edge function at '${func.path}' must be an object. More on the Edge Functions API at https://ntl.fyi/edge-api.`,
+        ),
+      )
+
       break
 
     case ConfigExitCode.SerializationError:
-      if (featureFlags.edge_functions_invalid_config_throw) {
-        throw new BundleError(
-          new Error(
-            `The 'config' object in the edge function at '${func.path}' must contain primitive values only. More on the Edge Functions API at https://ntl.fyi/edge-api.`,
-          ),
-        )
-      } else {
-        log.user(`'config' object in edge function at '${func.path}' must contain primitive values only`)
-      }
+      throw new BundleError(
+        new Error(
+          `The 'config' object in the edge function at '${func.path}' must contain primitive values only. More on the Edge Functions API at https://ntl.fyi/edge-api.`,
+        ),
+      )
       break
 
     case ConfigExitCode.InvalidDefaultExport:

diff --git a/packages/edge-bundler/node/feature_flags.ts b/packages/edge-bundler/node/feature_flags.ts
@@ -1,7 +1,6 @@
 const defaultFlags = {
   edge_functions_correct_order: false,
   edge_functions_fail_unsupported_regex: false,
-  edge_functions_invalid_config_throw: false,
 }
 
 type FeatureFlag = keyof typeof defaultFlags

diff --git a/packages/edge-bundler/node/server/server.ts b/packages/edge-bundler/node/server/server.ts
@@ -87,7 +87,7 @@ const prepareServer = ({
     let functionsConfig: FunctionConfig[] = []
 
     if (options.getFunctionsConfig) {
-      functionsConfig = await Promise.all(functions.map((func) => getFunctionConfig(func, importMap, deno, logger, {})))
+      functionsConfig = await Promise.all(functions.map((func) => getFunctionConfig(func, importMap, deno, logger)))
     }
 
     const success = await waitForServer(port, processRef.ps)