-
Notifications
You must be signed in to change notification settings - Fork 10
feat: implement functions timeout defaults and overrides in dev #346
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
Changes from all commits
b3a0d2e
06806c6
6a8f71f
5f61ad6
de3f0eb
945ab24
9f7d20f
38447dc
7cc60e4
fbdb989
3f9e135
95ced4a
cf256e8
4c179b7
08fd8fd
f475508
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,152 @@ | ||
| import { describe, expect, test } from 'vitest' | ||
|
|
||
| import { SYNCHRONOUS_FUNCTION_TIMEOUT, BACKGROUND_FUNCTION_TIMEOUT } from '@netlify/functions' | ||
| import { FunctionsRegistry } from './registry.js' | ||
|
|
||
| describe('FunctionsRegistry timeout configuration', () => { | ||
| test('uses default timeouts when no config or override provided', () => { | ||
| const registry = new FunctionsRegistry({ | ||
| config: {}, | ||
| destPath: '/tmp/test', | ||
| projectRoot: '/tmp/project', | ||
| settings: {}, | ||
| }) | ||
|
|
||
| expect(registry.timeouts).toEqual({ | ||
| syncFunctions: SYNCHRONOUS_FUNCTION_TIMEOUT, | ||
| backgroundFunctions: BACKGROUND_FUNCTION_TIMEOUT, | ||
| }) | ||
| }) | ||
|
|
||
| test('uses functions_timeout from siteInfo for sync functions only', () => { | ||
| const registry = new FunctionsRegistry({ | ||
| config: { | ||
| siteInfo: { | ||
| functions_timeout: 60, | ||
| }, | ||
| }, | ||
| destPath: '/tmp/test', | ||
| projectRoot: '/tmp/project', | ||
| settings: {}, | ||
| }) | ||
|
|
||
| expect(registry.timeouts).toEqual({ | ||
| syncFunctions: 60, | ||
| backgroundFunctions: BACKGROUND_FUNCTION_TIMEOUT, | ||
| }) | ||
| }) | ||
|
|
||
| test('uses functions_config.timeout from siteInfo for sync functions only', () => { | ||
| const registry = new FunctionsRegistry({ | ||
| config: { | ||
| siteInfo: { | ||
| functions_config: { | ||
| timeout: 45, | ||
| }, | ||
| }, | ||
| }, | ||
| destPath: '/tmp/test', | ||
| projectRoot: '/tmp/project', | ||
| settings: {}, | ||
| }) | ||
|
|
||
| expect(registry.timeouts).toEqual({ | ||
| syncFunctions: 45, | ||
| backgroundFunctions: BACKGROUND_FUNCTION_TIMEOUT, | ||
| }) | ||
| }) | ||
|
|
||
| test('prefers functions_timeout over functions_config.timeout for sync functions', () => { | ||
| const registry = new FunctionsRegistry({ | ||
| config: { | ||
| siteInfo: { | ||
| functions_timeout: 60, | ||
| functions_config: { | ||
| timeout: 45, | ||
| }, | ||
| }, | ||
| }, | ||
| destPath: '/tmp/test', | ||
| projectRoot: '/tmp/project', | ||
| settings: {}, | ||
| }) | ||
|
|
||
| expect(registry.timeouts).toEqual({ | ||
| syncFunctions: 60, | ||
| backgroundFunctions: BACKGROUND_FUNCTION_TIMEOUT, | ||
| }) | ||
| }) | ||
|
|
||
| test('uses override timeouts when provided', () => { | ||
| const registry = new FunctionsRegistry({ | ||
| config: { | ||
| siteInfo: { | ||
| functions_timeout: 60, | ||
| }, | ||
| }, | ||
| destPath: '/tmp/test', | ||
| projectRoot: '/tmp/project', | ||
| settings: {}, | ||
| timeouts: { | ||
| syncFunctions: 120, | ||
| backgroundFunctions: 1800, | ||
| }, | ||
| }) | ||
|
|
||
| expect(registry.timeouts).toEqual({ | ||
| syncFunctions: 120, | ||
| backgroundFunctions: 1800, | ||
| }) | ||
| }) | ||
|
|
||
| test('allows partial override of timeouts', () => { | ||
| const registry = new FunctionsRegistry({ | ||
| config: { | ||
| siteInfo: { | ||
| functions_timeout: 60, | ||
| }, | ||
| }, | ||
| destPath: '/tmp/test', | ||
| projectRoot: '/tmp/project', | ||
| settings: {}, | ||
| timeouts: { | ||
| syncFunctions: 120, | ||
| }, | ||
| }) | ||
|
|
||
| expect(registry.timeouts).toEqual({ | ||
| syncFunctions: 120, | ||
| backgroundFunctions: BACKGROUND_FUNCTION_TIMEOUT, | ||
| }) | ||
| }) | ||
|
|
||
| test('falls back to defaults when siteInfo is undefined', () => { | ||
| const registry = new FunctionsRegistry({ | ||
| config: { | ||
| siteInfo: undefined, | ||
| }, | ||
| destPath: '/tmp/test', | ||
| projectRoot: '/tmp/project', | ||
| settings: {}, | ||
| }) | ||
|
|
||
| expect(registry.timeouts).toEqual({ | ||
| syncFunctions: SYNCHRONOUS_FUNCTION_TIMEOUT, | ||
| backgroundFunctions: BACKGROUND_FUNCTION_TIMEOUT, | ||
| }) | ||
| }) | ||
|
|
||
| test('falls back to defaults when config is empty object', () => { | ||
| const registry = new FunctionsRegistry({ | ||
| config: {}, | ||
| destPath: '/tmp/test', | ||
| projectRoot: '/tmp/project', | ||
| settings: {}, | ||
| }) | ||
|
|
||
| expect(registry.timeouts).toEqual({ | ||
| syncFunctions: SYNCHRONOUS_FUNCTION_TIMEOUT, | ||
| backgroundFunctions: BACKGROUND_FUNCTION_TIMEOUT, | ||
| }) | ||
| }) | ||
| }) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ah ok, I thought we were missing the logic to pass from primitives/packages/functions/dev/src/main.ts Lines 33 to 34 in 95ced4a
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| import { describe, expect, test } from 'vitest' | ||
|
|
||
| import { BACKGROUND_FUNCTION_TIMEOUT, SYNCHRONOUS_FUNCTION_TIMEOUT } from './consts.js' | ||
|
|
||
| describe('Function timeout constants', () => { | ||
| test('exports correct timeout values', () => { | ||
| expect(SYNCHRONOUS_FUNCTION_TIMEOUT).toBe(30) | ||
| expect(BACKGROUND_FUNCTION_TIMEOUT).toBe(900) | ||
| }) | ||
| }) |
| Original file line number | Diff line number | Diff line change | ||
|---|---|---|---|---|
|
|
@@ -2,3 +2,4 @@ export type { Context } from './lib/context/context.js' | |||
| export type { Cookie } from './lib/context/cookies.js' | ||||
| export type { EnvironmentVariables } from './lib/environment-variables.js' | ||||
| export type { NetlifyGlobal } from './lib/globals.js' | ||||
| export type { Site } from './lib/context/site.js' | ||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. clean this up, we don't need it
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I guess it's fine... it's unrelated but it seems reasonable. |
||||
Uh oh!
There was an error while loading. Please reload this page.