Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: remove FF edge_functions_invalid_config_throw #374

Merged
merged 3 commits into from
May 5, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions node/bundler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
57 changes: 13 additions & 44 deletions node/config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'

Expand All @@ -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: {},
Expand All @@ -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")
Expand All @@ -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")
Expand All @@ -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',
Expand Down Expand Up @@ -171,7 +144,6 @@ describe('`getFunctionConfig` extracts configuration properties from function fi
new ImportMap([importMapFile]),
deno,
logger,
func.featureFlags || {},
)

if (func.error) {
Expand Down Expand Up @@ -270,7 +242,6 @@ test('Passes validation if default export exists and is a function', async () =>
new ImportMap([importMapFile]),
deno,
logger,
{},
),
).resolves.not.toThrow()

Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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))
Expand Down
67 changes: 21 additions & 46 deletions node/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -93,7 +85,7 @@ export const getFunctionConfig = async (
)

if (exitCode !== ConfigExitCode.Success) {
handleConfigError(func, exitCode, stderr, log, featureFlags)
handleConfigError(func, exitCode, stderr, log)

return {}
}
Expand All @@ -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()
}
Expand All @@ -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:
Expand All @@ -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:
Expand Down
1 change: 0 additions & 1 deletion node/feature_flags.ts
Original file line number Diff line number Diff line change
@@ -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
Expand Down
2 changes: 1 addition & 1 deletion node/server/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down