Skip to content

Commit

Permalink
feat: propagate onError config property to manifest (#328)
Browse files Browse the repository at this point in the history
* fix: throw errors when function or isc-config cannot be loaded

* chore: fix FF type

* feat: add on_error field

* :old-man-yells-at-linter:

* fix: prettier

* update snapshots

* feat: move on_error into per-function config

* fix: prettier, argh

* Update node/config.ts

Co-authored-by: Eduardo Bouças <mail@eduardoboucas.com>

* fix: test

---------

Co-authored-by: Daniel Tschinder <231804+danez@users.noreply.github.com>
Co-authored-by: Eduardo Bouças <mail@eduardoboucas.com>
  • Loading branch information
3 people committed Mar 8, 2023
1 parent 6a4c600 commit bd804b2
Show file tree
Hide file tree
Showing 5 changed files with 70 additions and 14 deletions.
18 changes: 18 additions & 0 deletions node/config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,24 @@ const functions = [
edge_functions_invalid_config_throw: false,
},
},
{
testName: 'config with correct onError',
expectedConfig: { onError: 'bypass' },
name: 'func5',
source: `
export default async () => new Response("Hello from function two")
export const config = { onError: "bypass" }
`,
},
{
testName: 'config with wrong onError',
name: 'func7',
source: `
export default async () => new Response("Hello from function two")
export const config = { onError: "foo" }
`,
error: /The 'onError' configuration property in edge function at .*/,
},
{
testName: 'config with `path`',
expectedConfig: { path: '/home' },
Expand Down
28 changes: 23 additions & 5 deletions node/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,19 @@ export const enum Cache {
Manual = 'manual',
}

export type OnError = 'fail' | 'bypass' | `/${string}`

export const isValidOnError = (value: unknown): value is OnError => {
if (typeof value === 'undefined') return true
if (typeof value !== 'string') return false
return value === 'fail' || value === 'bypass' || value.startsWith('/')
}

export interface FunctionConfig {
cache?: Cache
path?: string | string[]
excludedPath?: string | string[]
onError?: OnError
}

const getConfigExtractor = () => {
Expand Down Expand Up @@ -89,17 +98,26 @@ export const getFunctionConfig = async (
log.user(stdout)
}

try {
const collectorData = await fs.readFile(collector.path, 'utf8')
let collectorData: FunctionConfig = {}

return JSON.parse(collectorData) as FunctionConfig
try {
const collectorDataJSON = await fs.readFile(collector.path, 'utf8')
collectorData = JSON.parse(collectorDataJSON) as FunctionConfig
} catch {
handleConfigError(func, ConfigExitCode.UnhandledError, stderr, log, featureFlags)

return {}
} finally {
await collector.cleanup()
}

if (!isValidOnError(collectorData.onError)) {
throw new BundleError(
new Error(
`The 'onError' configuration property in edge function at '${func.path}' must be one of 'fail', 'bypass', or a path starting with '/'. Got '${collectorData.onError}'. More on the Edge Functions API at https://ntl.fyi/edge-api.`,
),
)
}

return collectorData
}

const handleConfigError = (
Expand Down
21 changes: 21 additions & 0 deletions node/manifest.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { env } from 'process'
import { test, expect, vi } from 'vitest'

import { BundleFormat } from './bundle.js'
import { FunctionConfig } from './config.js'
import { Declaration } from './declaration.js'
import { generateManifest } from './manifest.js'

Expand Down Expand Up @@ -101,6 +102,26 @@ test('Generates a manifest with excluded paths and patterns', () => {
expect(manifest.bundler_version).toBe(env.npm_package_version as string)
})

test('Includes failure modes in manifest', () => {
const functions = [
{ name: 'func-1', path: '/path/to/func-1.ts' },
{ name: 'func-2', path: '/path/to/func-2.ts' },
]
const declarations: Declaration[] = [
{ function: 'func-1', name: 'Display Name', path: '/f1/*' },
{ function: 'func-2', pattern: '^/f2/.*/?$' },
]
const functionConfig: Record<string, FunctionConfig> = {
'func-1': {
onError: '/custom-error',
},
}
const manifest = generateManifest({ bundles: [], declarations, functions, functionConfig })
expect(manifest.function_config).toEqual({
'func-1': { excluded_patterns: [], on_error: '/custom-error' },
})
})

test('Excludes functions for which there are function files but no matching config declarations', () => {
const bundle1 = {
extension: '.ext2',
Expand Down
16 changes: 7 additions & 9 deletions node/manifest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ interface Route {
}
interface EdgeFunctionConfig {
excluded_patterns: string[]
on_error?: string
}
interface Manifest {
bundler_version: string
Expand All @@ -41,13 +42,6 @@ interface GenerateManifestOptions {
layers?: Layer[]
}

interface Route {
function: string
name?: string
pattern: string
generator?: string
}

// JavaScript regular expressions are converted to strings with leading and
// trailing slashes, so any slashes inside the expression itself are escaped
// as `//`. This function deserializes that back into a single slash, which
Expand All @@ -58,7 +52,7 @@ const sanitizeEdgeFunctionConfig = (config: Record<string, EdgeFunctionConfig>):
const newConfig: Record<string, EdgeFunctionConfig> = {}

for (const [name, functionConfig] of Object.entries(config)) {
if (functionConfig.excluded_patterns.length !== 0) {
if (functionConfig.excluded_patterns.length !== 0 || functionConfig.on_error) {
newConfig[name] = functionConfig
}
}
Expand All @@ -81,13 +75,17 @@ const generateManifest = ({
functions.map(({ name }) => [name, { excluded_patterns: [] }]),
)

for (const [name, { excludedPath }] of Object.entries(functionConfig)) {
for (const [name, { excludedPath, onError }] of Object.entries(functionConfig)) {
if (excludedPath) {
const paths = Array.isArray(excludedPath) ? excludedPath : [excludedPath]
const excludedPatterns = paths.map(pathToRegularExpression).map(serializePattern)

manifestFunctionConfig[name].excluded_patterns.push(...excludedPatterns)
}

if (onError) {
manifestFunctionConfig[name].on_error = onError
}
}

declarations.forEach((declaration) => {
Expand Down
1 change: 1 addition & 0 deletions node/validation/manifest/schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ const functionConfigSchema = {
'excluded_patterns needs to be an array of regex that starts with ^ and ends with $ without any additional slashes before and afterwards',
},
},
on_error: { type: 'string' },
},
}

Expand Down

0 comments on commit bd804b2

Please sign in to comment.