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: functions:serve should serve V2 Functions #6290

Merged
merged 25 commits into from
Jan 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
6a1378c
refactor: type server.ts
Skn0tt Jan 9, 2024
0d3db63
chore: write acceptance test
Skn0tt Jan 9, 2024
0c9c3d1
refactor: replace Object.assign with spread
Skn0tt Jan 9, 2024
e5466ca
refactor: even more types
Skn0tt Jan 9, 2024
d5ef10b
fix: pass blobs context into server
Skn0tt Jan 9, 2024
d441757
fix: some more typescript types
Skn0tt Jan 9, 2024
27dfc13
fix: remove unneeded parameter pass
Skn0tt Jan 9, 2024
08f001d
fix: perform v2 function matching in functions server
Skn0tt Jan 9, 2024
a5ba20e
refactor: some more types
Skn0tt Jan 9, 2024
a7871cb
fix: remove test.only
Skn0tt Jan 9, 2024
88f51da
fix: prettier
Skn0tt Jan 9, 2024
cdd1735
fix: buildData can be undefined
Skn0tt Jan 9, 2024
210b540
fix: other test
Skn0tt Jan 9, 2024
35ea491
fix: handler unit test
Skn0tt Jan 10, 2024
dc4c91f
fix: builddata doesnt always exist
Skn0tt Jan 10, 2024
8658416
Merge branch 'main' into functions-serve-v2-functions
Skn0tt Jan 10, 2024
5960cab
Merge branch 'main' into functions-serve-v2-functions
Skn0tt Jan 10, 2024
42b1154
Merge branch 'main' into functions-serve-v2-functions
Skn0tt Jan 11, 2024
fe9f500
Merge branch 'main' into functions-serve-v2-functions
Skn0tt Jan 16, 2024
7639863
Update src/lib/functions/server.ts
Skn0tt Jan 18, 2024
63c5d93
Merge branch 'main' into functions-serve-v2-functions
Skn0tt Jan 18, 2024
17cfcaf
fix: use constant for site mock id
Skn0tt Jan 18, 2024
67abd2e
chore: format
Skn0tt Jan 18, 2024
2fe2ffd
Merge branch 'main' into functions-serve-v2-functions
Skn0tt Jan 18, 2024
329ffdb
Merge branch 'main' into functions-serve-v2-functions
Skn0tt Jan 18, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion src/commands/dev/dev.ts
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,6 @@ export const dev = async (options: OptionValues, command: BaseCommand) => {
})

const functionsRegistry = await startFunctionsServer({
api,
blobsContext,
command,
config,
Expand Down
19 changes: 15 additions & 4 deletions src/commands/functions/functions-serve.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,16 @@ import { join } from 'path'

import { OptionValues } from 'commander'

import { getBlobsContext } from '../../lib/blobs/blobs.js'
import { startFunctionsServer } from '../../lib/functions/server.js'
import { printBanner } from '../../utils/banner.js'
import { acquirePort, getDotEnvVariables, getSiteInformation, injectEnvVariables } from '../../utils/dev.js'
import {
UNLINKED_SITE_MOCK_ID,
acquirePort,
getDotEnvVariables,
getSiteInformation,
injectEnvVariables,
} from '../../utils/dev.js'
import { getFunctionsDir } from '../../utils/functions/index.js'
import { getProxyUrl } from '../../utils/proxy.js'
import BaseCommand from '../base-command.js'
Expand Down Expand Up @@ -35,19 +42,23 @@ export const functionsServe = async (options: OptionValues, command: BaseCommand
errorMessage: 'Could not acquire configured functions port',
})

const blobsContext = await getBlobsContext({
debug: options.debug,
projectRoot: command.workingDir,
siteID: site.id ?? UNLINKED_SITE_MOCK_ID,
})

await startFunctionsServer({
blobsContext,
config,
debug: options.debug,
command,
api,
settings: { functions: functionsDir, functionsPort },
site,
siteInfo,
siteUrl,
capabilities,
timeouts,
functionsPrefix: '/.netlify/functions/',
buildersPrefix: '/.netlify/builders/',
geolocationMode: options.geo,
geoCountry: options.country,
offline: options.offline,
Expand Down
1 change: 0 additions & 1 deletion src/commands/serve/serve.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,6 @@ export const serve = async (options: OptionValues, command: BaseCommand) => {
})

const functionsRegistry = await startFunctionsServer({
api,
blobsContext,
command,
config,
Expand Down
17 changes: 12 additions & 5 deletions src/lib/functions/form-submissions-handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { Readable } from 'stream'

// @ts-expect-error TS(7016) FIXME: Could not find a declaration file for module 'cont... Remove this comment to see the full error message
import { parse as parseContentType } from 'content-type'
import type { RequestHandler } from 'express'
// @ts-expect-error TS(7016) FIXME: Could not find a declaration file for module 'mult... Remove this comment to see the full error message
import multiparty from 'multiparty'
import getRawBody from 'raw-body'
Expand All @@ -10,6 +11,8 @@ import { warn } from '../../utils/command-helpers.js'
import { BACKGROUND } from '../../utils/functions/index.js'
import { capitalize } from '../string.js'

import type { FunctionsRegistry } from './registry.js'

// @ts-expect-error TS(7031) FIXME: Binding element 'functionsRegistry' implicitly has... Remove this comment to see the full error message
const getFormHandler = function ({ functionsRegistry }) {
const handlers = ['submission-created', `submission-created${BACKGROUND}`]
Expand All @@ -29,14 +32,18 @@ const getFormHandler = function ({ functionsRegistry }) {
return handlers[0]
}

// @ts-expect-error TS(7031) FIXME: Binding element 'functionsRegistry' implicitly has... Remove this comment to see the full error message
export const createFormSubmissionHandler = function ({ functionsRegistry, siteUrl }) {
// @ts-expect-error TS(7006) FIXME: Parameter 'req' implicitly has an 'any' type.
export const createFormSubmissionHandler = function ({
functionsRegistry,
siteUrl,
}: {
functionsRegistry: FunctionsRegistry
siteUrl: string
}): RequestHandler {
return async function formSubmissionHandler(req, res, next) {
if (
req.url.startsWith('/.netlify/') ||
req.method !== 'POST' ||
(await functionsRegistry.getFunctionForURLPath(req.url, req.method))
(await functionsRegistry.getFunctionForURLPath(req.url, req.method, () => Promise.resolve(false)))
)
return next()

Expand Down Expand Up @@ -159,7 +166,7 @@ export const createFormSubmissionHandler = function ({ functionsRegistry, siteUr
req.body = data
req.headers = {
...req.headers,
'content-length': data.length,
'content-length': String(data.length),
'content-type': 'application/json',
'x-netlify-original-pathname': originalUrl.pathname,
'x-netlify-original-search': originalUrl.search,
Expand Down
4 changes: 2 additions & 2 deletions src/lib/functions/netlify-function.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ export default class NetlifyFunction {

// Determines whether this is a background function based on the function
// name.
private readonly isBackground: boolean
public readonly isBackground: boolean

private buildQueue?: Promise<$FIXME>
private buildData?: $FIXME
Expand Down Expand Up @@ -270,7 +270,7 @@ export default class NetlifyFunction {

let path = rawPath !== '/' && rawPath.endsWith('/') ? rawPath.slice(0, -1) : rawPath
path = path.toLowerCase()
const { routes = [] } = this.buildData
const { routes = [] } = this.buildData ?? {}
// @ts-expect-error TS(7031) FIXME: Binding element 'expression' implicitly has an 'an... Remove this comment to see the full error message
const route = routes.find(({ expression, literal, methods }) => {
if (methods.length !== 0 && !methods.includes(method)) {
Expand Down
2 changes: 1 addition & 1 deletion src/lib/functions/registry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,7 @@ export class FunctionsRegistry {
return { func: null, route: null }
}

const { routes = [] } = await func.getBuildData()
const { routes = [] } = (await func.getBuildData()) ?? {}

if (routes.length !== 0) {
// @ts-expect-error TS(7006) FIXME: Parameter 'route' implicitly has an 'any' type.
Expand Down
127 changes: 69 additions & 58 deletions src/lib/functions/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,13 @@ import { Buffer } from 'buffer'
import { promises as fs } from 'fs'
import path from 'path'

import express from 'express'
import express, { type RequestHandler } from 'express'
// @ts-expect-error TS(7016) FIXME: Could not find a declaration file for module 'expr... Remove this comment to see the full error message
import expressLogging from 'express-logging'
import jwtDecode from 'jwt-decode'

import type BaseCommand from '../../commands/base-command.js'
import type { $TSFixMe } from '../../commands/types.js'
import { NETLIFYDEVERR, NETLIFYDEVLOG, error as errorExit, log } from '../../utils/command-helpers.js'
import { isFeatureFlagEnabled } from '../../utils/feature-flags.js'
import {
Expand All @@ -16,6 +18,7 @@ import {
getInternalFunctionsDir,
} from '../../utils/functions/index.js'
import { NFFunctionName, NFFunctionRoute } from '../../utils/headers.js'
import type { BlobsContext } from '../blobs/blobs.js'
import { headers as efHeaders } from '../edge-functions/headers.js'
import { getGeoLocation } from '../geo-location.js'

Expand Down Expand Up @@ -78,11 +81,9 @@ const hasBody = (req) =>
// we expect a string or a buffer, because we use the two bodyParsers(text, raw) from express
(typeof req.body === 'string' || Buffer.isBuffer(req.body))

// @ts-expect-error TS(7006) FIXME: Parameter 'options' implicitly has an 'any' type.
export const createHandler = function (options) {
export const createHandler = function (options: GetFunctionsServerOptions): RequestHandler {
const { functionsRegistry } = options

// @ts-expect-error TS(7006) FIXME: Parameter 'request' implicitly has an 'any' type.
return async function handler(request, response) {
// If these headers are set, it means we've already matched a function and we
// can just grab its name directly. We delete the header from the request
Expand All @@ -92,15 +93,22 @@ export const createHandler = function (options) {
const functionRoute = request.header(NFFunctionRoute)
delete request.headers[NFFunctionRoute]

// If we didn't match a function with a custom route, let's try to match
// using the fixed URL format.
// If there's still no function found, we check the functionsRegistry again.
// This is needed for the functions:serve command, where the dev server that normally does the matching doesn't run.
// It also matches the default URL (.netlify/functions/builders)
if (!functionName) {
const cleanPath = request.path.replace(/^\/.netlify\/(functions|builders)/, '')

functionName = cleanPath.split('/').find(Boolean)
const match = await functionsRegistry.getFunctionForURLPath(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the main functional change, all the rest is cleanup.

request.url,
request.method,
// we're pretending there's no static file at the same URL.
// This is wrong, but in local dev we already did the matching
// in a downstream server where we had access to the file system, so this never hits.
() => Promise.resolve(false),
)
functionName = match?.func?.name
}

const func = functionsRegistry.get(functionName)
const func = functionsRegistry.get(functionName ?? '')

if (func === undefined) {
response.statusCode = 404
Expand All @@ -121,16 +129,14 @@ export const createHandler = function (options) {
}

let remoteAddress = request.header('x-forwarded-for') || request.connection.remoteAddress || ''
remoteAddress = remoteAddress
.split(remoteAddress.includes('.') ? ':' : ',')
.pop()
.trim()

let requestPath = request.path
if (request.header('x-netlify-original-pathname')) {
requestPath = request.header('x-netlify-original-pathname')
delete request.headers['x-netlify-original-pathname']
}
remoteAddress =
remoteAddress
.split(remoteAddress.includes('.') ? ':' : ',')
.pop()
?.trim() ?? ''

const requestPath = request.header('x-netlify-original-pathname') ?? request.path
delete request.headers['x-netlify-original-pathname']

let requestQuery = request.query
if (request.header('x-netlify-original-search')) {
Expand All @@ -151,7 +157,7 @@ export const createHandler = function (options) {
{},
)

const geoLocation = await getGeoLocation({ ...options, mode: options.geo })
const geoLocation = await getGeoLocation({ ...options, mode: options.geolocationMode })

const headers = Object.entries({
...request.headers,
Expand All @@ -161,7 +167,7 @@ export const createHandler = function (options) {
'x-nf-site-id': [options?.siteInfo?.id ?? UNLINKED_SITE_MOCK_ID],
[efHeaders.Geo]: Buffer.from(JSON.stringify(geoLocation)).toString('base64'),
}).reduce((prev, [key, value]) => ({ ...prev, [key]: Array.isArray(value) ? value : [value] }), {})
const rawQuery = new URLSearchParams(requestQuery).toString()
const rawQuery = new URLSearchParams(requestQuery as Record<string, string>).toString()
const protocol = options.config?.dev?.https ? 'https' : 'http'
const url = new URL(requestPath, `${protocol}://${request.get('host') || 'localhost'}`)
url.search = rawQuery
Expand Down Expand Up @@ -235,9 +241,20 @@ export const createHandler = function (options) {
}
}

// @ts-expect-error TS(7006) FIXME: Parameter 'options' implicitly has an 'any' type.
const getFunctionsServer = (options) => {
const { buildersPrefix = '', functionsPrefix = '', functionsRegistry, siteUrl } = options
interface GetFunctionsServerOptions {
functionsRegistry: FunctionsRegistry
siteUrl: string
siteInfo?: $TSFixMe
accountId: string
geoCountry: string
offline: boolean
state: $TSFixMe
config: $TSFixMe
geolocationMode: 'cache' | 'update' | 'mock'
}

const getFunctionsServer = (options: GetFunctionsServerOptions) => {
const { functionsRegistry, siteUrl } = options
const app = express()
const functionHandler = createHandler(options)

Expand All @@ -257,30 +274,25 @@ const getFunctionsServer = (options) => {
}),
)

app.all(`${functionsPrefix}*`, functionHandler)
app.all(`${buildersPrefix}*`, functionHandler)
app.all('*', functionHandler)

return app
}

/**
*
* @param {object} options
* @param {import("../blobs/blobs.js").BlobsContext} options.blobsContext
* @param {import('../../commands/base-command.js').default} options.command
* @param {*} options.capabilities
* @param {*} options.config
* @param {boolean} options.debug
* @param {*} options.loadDistFunctions
* @param {*} options.settings
* @param {*} options.site
* @param {*} options.siteInfo
* @param {string} options.siteUrl
* @param {*} options.timeouts
* @returns {Promise<import('./registry.js').FunctionsRegistry | undefined>}
*/
// @ts-expect-error TS(7006) FIXME: Parameter 'options' implicitly has an 'any' type.
export const startFunctionsServer = async (options) => {
export const startFunctionsServer = async (
options: {
blobsContext: BlobsContext
command: BaseCommand
config: $TSFixMe
capabilities: $TSFixMe
debug: boolean
loadDistFunctions?: boolean
settings: $TSFixMe
site: $TSFixMe
siteInfo: $TSFixMe
timeouts: $TSFixMe
} & Omit<GetFunctionsServerOptions, 'functionsRegistry'>,
): Promise<FunctionsRegistry | undefined> => {
const {
blobsContext,
capabilities,
Expand Down Expand Up @@ -356,31 +368,30 @@ export const startFunctionsServer = async (options) => {

await functionsRegistry.scan(functionsDirectories)

const server = getFunctionsServer(Object.assign(options, { functionsRegistry }))
const server = getFunctionsServer({ ...options, functionsRegistry })

await startWebServer({ server, settings, debug })

return functionsRegistry
}

/**
*
* @param {object} config
* @param {boolean} config.debug
* @param {ReturnType<Awaited<typeof getFunctionsServer>>} config.server
* @param {*} config.settings
*/
// @ts-expect-error TS(7031) FIXME: Binding element 'debug' implicitly has an 'any' ty... Remove this comment to see the full error message
const startWebServer = async ({ debug, server, settings }) => {
await new Promise((/** @type {(resolve: void) => void} */ resolve) => {
const startWebServer = async ({
debug,
server,
settings,
}: {
debug: boolean
server: ReturnType<Awaited<typeof getFunctionsServer>>
settings: $TSFixMe
}) => {
await new Promise<void>((resolve) => {
// @ts-expect-error TS(7006) FIXME: Parameter 'err' implicitly has an 'any' type.
server.listen(settings.functionsPort, (/** @type {unknown} */ err) => {
server.listen(settings.functionsPort, (err) => {
if (err) {
errorExit(`${NETLIFYDEVERR} Unable to start functions server: ${err}`)
} else if (debug) {
log(`${NETLIFYDEVLOG} Functions server is listening on ${settings.functionsPort}`)
}
// @ts-expect-error TS(2794) FIXME: Expected 1 arguments, but got 0. Did you forget to... Remove this comment to see the full error message
resolve()
})
})
Expand Down
4 changes: 2 additions & 2 deletions tests/integration/commands/dev/dev-miscellaneous.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -719,7 +719,7 @@ describe.concurrent('commands/dev-miscellaneous', () => {
})

test(`catches invalid function names`, async (t) => {
await withSiteBuilder('site-with-functions', async (builder) => {
await withSiteBuilder(t, async (builder) => {
const functionsPort = 6667
await builder
.withNetlifyToml({ config: { functions: { directory: 'functions' }, dev: { functionsPort } } })
Expand All @@ -733,7 +733,7 @@ describe.concurrent('commands/dev-miscellaneous', () => {
.buildAsync()

await withDevServer({ cwd: builder.directory }, async ({ port, url }) => {
const response = await fetch(`${url.replace(port, functionsPort)}/exclamat!on`, {
const response = await fetch(`${url.replace(port, functionsPort)}/.netlify/functions/exclamat!on`, {
method: 'POST',
headers: {
'Content-Type': 'application/json',
Expand Down
Loading
Loading