From abfdf60030543237ce7e614fa40d89df858d5742 Mon Sep 17 00:00:00 2001 From: hossam-nasr Date: Fri, 22 Apr 2022 19:43:50 -0700 Subject: [PATCH 01/39] add startup and teardown hook support --- src/WorkerChannel.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/WorkerChannel.ts b/src/WorkerChannel.ts index 7225490c..4811cc69 100644 --- a/src/WorkerChannel.ts +++ b/src/WorkerChannel.ts @@ -17,6 +17,8 @@ export class WorkerChannel { packageJson: PackageJson; #preInvocationHooks: HookCallback[] = []; #postInvocationHooks: HookCallback[] = []; + #appStartupHooks: HookCallback[] = []; + #appTeardownHooks: HookCallback[] = []; constructor(eventStream: IEventStream, functionLoader: IFunctionLoader) { this.eventStream = eventStream; @@ -80,6 +82,10 @@ export class WorkerChannel { return this.#preInvocationHooks; case 'postInvocation': return this.#postInvocationHooks; + case 'appStartup': + return this.#appStartupHooks; + case 'appTeardown': + return this.#appTeardownHooks; default: throw new RangeError(`Unrecognized hook "${hookName}"`); } From 3cd4888e07eef10db3b5a18e546b6ff5513a2057 Mon Sep 17 00:00:00 2001 From: hossam-nasr Date: Fri, 22 Apr 2022 19:43:58 -0700 Subject: [PATCH 02/39] add initial types --- types-core/index.d.ts | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/types-core/index.d.ts b/types-core/index.d.ts index 736b8386..b291cd21 100644 --- a/types-core/index.d.ts +++ b/types-core/index.d.ts @@ -18,11 +18,15 @@ declare module '@azure/functions-core' { */ export function registerHook(hookName: 'preInvocation', callback: PreInvocationCallback): Disposable; export function registerHook(hookName: 'postInvocation', callback: PostInvocationCallback): Disposable; + export function registerHook(hookName: 'appStartup', callback: AppStartupCallback): Disposable; + export function registerHook(hookName: 'appTeardown', callback: AppTeardownCallback): Disposable; export function registerHook(hookName: string, callback: HookCallback): Disposable; export type HookCallback = (context: HookContext) => void | Promise; export type PreInvocationCallback = (context: PreInvocationContext) => void | Promise; export type PostInvocationCallback = (context: PostInvocationContext) => void | Promise; + export type AppStartupCallback = (context: AppStartupContext) => void | Promise; + export type AppTeardownCallback = (context: AppTeardownCallback) => void | Promise; export type HookData = { [key: string]: any }; @@ -83,6 +87,22 @@ declare module '@azure/functions-core' { error: any; } + /** + * Context on a function app that is about to be started + * This object will be passed to all app startup hooks + */ + export interface AppStartupContext extends HookContext { + something: any; + } + + /** + * Context on a function app that has is about to be deallocated + * This object will be passed to all app teardown hooks + */ + export interface AppTeardownContext extends HookContext { + otherThing: any; + } + /** * Represents a type which can release resources, such as event listening or a timer. */ From 4189d7f2175dece55e8fe1f93f84d53ee8be443e Mon Sep 17 00:00:00 2001 From: hossam-nasr Date: Mon, 25 Apr 2022 08:33:05 -0700 Subject: [PATCH 03/39] move hook data handling to wokrer channel --- src/WorkerChannel.ts | 4 +++- src/eventHandlers/InvocationHandler.ts | 7 +++---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/WorkerChannel.ts b/src/WorkerChannel.ts index 4811cc69..358c7b5d 100644 --- a/src/WorkerChannel.ts +++ b/src/WorkerChannel.ts @@ -1,7 +1,7 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the MIT License. -import { HookCallback, HookContext } from '@azure/functions-core'; +import { HookCallback, HookContext, HookData } from '@azure/functions-core'; import { AzureFunctionsRpcMessages as rpc } from '../azure-functions-language-worker-protobuf/src/rpc'; import { Disposable } from './Disposable'; import { IFunctionLoader } from './FunctionLoader'; @@ -15,6 +15,7 @@ export class WorkerChannel { eventStream: IEventStream; functionLoader: IFunctionLoader; packageJson: PackageJson; + #hookData: HookData = {}; #preInvocationHooks: HookCallback[] = []; #postInvocationHooks: HookCallback[] = []; #appStartupHooks: HookCallback[] = []; @@ -63,6 +64,7 @@ export class WorkerChannel { invocationId, category: msgCategory, }); + context.hookData = this.#hookData; for (const callback of callbacks) { await callback(context); } diff --git a/src/eventHandlers/InvocationHandler.ts b/src/eventHandlers/InvocationHandler.ts index 8049ea7d..82eb3087 100644 --- a/src/eventHandlers/InvocationHandler.ts +++ b/src/eventHandlers/InvocationHandler.ts @@ -2,7 +2,7 @@ // Licensed under the MIT License. import { AzureFunction } from '@azure/functions'; -import { HookData, PostInvocationContext, PreInvocationContext } from '@azure/functions-core'; +import { PostInvocationContext, PreInvocationContext } from '@azure/functions-core'; import { format } from 'util'; import { AzureFunctionsRpcMessages as rpc } from '../../azure-functions-language-worker-protobuf/src/rpc'; import { CreateContextAndInputs } from '../Context'; @@ -90,10 +90,9 @@ export class InvocationHandler extends EventHandler<'invocationRequest', 'invoca }); }); - const hookData: HookData = {}; let userFunction = channel.functionLoader.getFunc(functionId); const preInvocContext: PreInvocationContext = { - hookData, + hookData: {}, invocationContext: context, functionCallback: userFunction, inputs, @@ -117,7 +116,7 @@ export class InvocationHandler extends EventHandler<'invocationRequest', 'invoca } const postInvocContext: PostInvocationContext = { - hookData, + hookData: {}, invocationContext: context, inputs, result: null, From 5b778f58abb3b2ca159e0f0bcf03e13d2d531176 Mon Sep 17 00:00:00 2001 From: hossam-nasr Date: Mon, 25 Apr 2022 09:11:15 -0700 Subject: [PATCH 04/39] add app startup hooks to worker init handler --- src/eventHandlers/WorkerInitHandler.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/eventHandlers/WorkerInitHandler.ts b/src/eventHandlers/WorkerInitHandler.ts index 45ba230c..341f98a5 100644 --- a/src/eventHandlers/WorkerInitHandler.ts +++ b/src/eventHandlers/WorkerInitHandler.ts @@ -1,6 +1,7 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the MIT License. +import { AppStartupContext } from '@azure/functions-core'; import { access, constants } from 'fs'; import * as path from 'path'; import { AzureFunctionsRpcMessages as rpc } from '../../azure-functions-language-worker-protobuf/src/rpc'; @@ -35,6 +36,12 @@ export class WorkerInitHandler extends EventHandler<'workerInitRequest', 'worker await channel.updatePackageJson(functionAppDirectory); } + const context: AppStartupContext = { + hookData: {}, + something: '', + }; + await channel.executeHooks('appStartup', context); + response.capabilities = { RpcHttpTriggerMetadataRemoved: 'true', RpcHttpBodyOnly: 'true', From b94a3bac4bdf9d88804c7344eb9a87d7b348fadf Mon Sep 17 00:00:00 2001 From: hossam-nasr Date: Mon, 25 Apr 2022 09:27:00 -0700 Subject: [PATCH 05/39] run new script file on functionEnvironmentReloadRequest --- src/WorkerChannel.ts | 36 ++++++++++++++++++- .../FunctionEnvironmentReloadHandler.ts | 2 +- 2 files changed, 36 insertions(+), 2 deletions(-) diff --git a/src/WorkerChannel.ts b/src/WorkerChannel.ts index 358c7b5d..48db33ba 100644 --- a/src/WorkerChannel.ts +++ b/src/WorkerChannel.ts @@ -2,12 +2,15 @@ // Licensed under the MIT License. import { HookCallback, HookContext, HookData } from '@azure/functions-core'; +import { pathExists } from 'fs-extra'; import { AzureFunctionsRpcMessages as rpc } from '../azure-functions-language-worker-protobuf/src/rpc'; import { Disposable } from './Disposable'; import { IFunctionLoader } from './FunctionLoader'; import { IEventStream } from './GrpcClient'; +import { loadScriptFile } from './loadScriptFile'; import { PackageJson, parsePackageJson } from './parsers/parsePackageJson'; import { ensureErrorType } from './utils/ensureErrorType'; +import path = require('path'); import LogLevel = rpc.RpcLog.Level; import LogCategory = rpc.RpcLog.RpcLogCategory; @@ -93,7 +96,38 @@ export class WorkerChannel { } } - async updatePackageJson(dir: string): Promise { + async updateFunctionAppDirectory(functionAppDirectory: string): Promise { + await this.#updatePackageJson(functionAppDirectory); + + const entryPointFile = this.packageJson.main; + if (entryPointFile) { + this.log({ + message: `Loading entry point "${entryPointFile}"`, + level: LogLevel.Debug, + logCategory: LogCategory.System, + }); + try { + const entryPointFullPath = path.join(functionAppDirectory, entryPointFile); + if (!(await pathExists(entryPointFullPath))) { + throw new Error(`file does not exist`); + } + + await loadScriptFile(entryPointFullPath, this.packageJson); + this.log({ + message: `Loaded entry point "${entryPointFile}"`, + level: LogLevel.Debug, + logCategory: LogCategory.System, + }); + } catch (err) { + const error = ensureErrorType(err); + error.isAzureFunctionsInternalException = true; + error.message = `Worker was unable to load entry point "${entryPointFile}": ${error.message}`; + throw error; + } + } + } + + async #updatePackageJson(dir: string): Promise { try { this.packageJson = await parsePackageJson(dir); } catch (err) { diff --git a/src/eventHandlers/FunctionEnvironmentReloadHandler.ts b/src/eventHandlers/FunctionEnvironmentReloadHandler.ts index ccf14b2a..7a4ca37e 100644 --- a/src/eventHandlers/FunctionEnvironmentReloadHandler.ts +++ b/src/eventHandlers/FunctionEnvironmentReloadHandler.ts @@ -43,7 +43,7 @@ export class FunctionEnvironmentReloadHandler extends EventHandler< logCategory: LogCategory.System, }); process.chdir(msg.functionAppDirectory); - await channel.updatePackageJson(msg.functionAppDirectory); + await channel.updateFunctionAppDirectory(msg.functionAppDirectory); } return response; From 9a099b2c04de3b2cbda1cbf59ac5c610b3054230 Mon Sep 17 00:00:00 2001 From: hossam-nasr Date: Mon, 25 Apr 2022 14:26:50 -0700 Subject: [PATCH 06/39] add logger to base hook --- src/WorkerChannel.ts | 32 ++++++++++++++++++++++++++ src/eventHandlers/InvocationHandler.ts | 4 ++-- src/eventHandlers/WorkerInitHandler.ts | 2 +- types-core/index.d.ts | 8 +++++++ 4 files changed, 43 insertions(+), 3 deletions(-) diff --git a/src/WorkerChannel.ts b/src/WorkerChannel.ts index 48db33ba..b9b2869c 100644 --- a/src/WorkerChannel.ts +++ b/src/WorkerChannel.ts @@ -81,6 +81,38 @@ export class WorkerChannel { } } + getBaseHookContext(): HookContext { + return { + hookData: this.#hookData, + logger: { + log: (msg: string) => { + this.log({ + category: 'executionHooksLog', + message: msg, + level: LogLevel.Debug, + logCategory: LogCategory.User, + }); + }, + warn: (msg: string) => { + this.log({ + category: 'executionHooksWarn', + message: msg, + level: LogLevel.Warning, + logCategory: LogCategory.User, + }); + }, + error: (msg: string) => { + this.log({ + category: 'executionHooksError', + message: msg, + level: LogLevel.Error, + logCategory: LogCategory.User, + }); + }, + }, + }; + } + #getHooks(hookName: string): HookCallback[] { switch (hookName) { case 'preInvocation': diff --git a/src/eventHandlers/InvocationHandler.ts b/src/eventHandlers/InvocationHandler.ts index 82eb3087..19a18b32 100644 --- a/src/eventHandlers/InvocationHandler.ts +++ b/src/eventHandlers/InvocationHandler.ts @@ -92,7 +92,7 @@ export class InvocationHandler extends EventHandler<'invocationRequest', 'invoca let userFunction = channel.functionLoader.getFunc(functionId); const preInvocContext: PreInvocationContext = { - hookData: {}, + ...channel.getBaseHookContext(), invocationContext: context, functionCallback: userFunction, inputs, @@ -116,7 +116,7 @@ export class InvocationHandler extends EventHandler<'invocationRequest', 'invoca } const postInvocContext: PostInvocationContext = { - hookData: {}, + ...channel.getBaseHookContext(), invocationContext: context, inputs, result: null, diff --git a/src/eventHandlers/WorkerInitHandler.ts b/src/eventHandlers/WorkerInitHandler.ts index 341f98a5..1ec21a02 100644 --- a/src/eventHandlers/WorkerInitHandler.ts +++ b/src/eventHandlers/WorkerInitHandler.ts @@ -37,7 +37,7 @@ export class WorkerInitHandler extends EventHandler<'workerInitRequest', 'worker } const context: AppStartupContext = { - hookData: {}, + ...channel.getBaseHookContext(), something: '', }; await channel.executeHooks('appStartup', context); diff --git a/types-core/index.d.ts b/types-core/index.d.ts index b291cd21..7b64cce2 100644 --- a/types-core/index.d.ts +++ b/types-core/index.d.ts @@ -38,6 +38,14 @@ declare module '@azure/functions-core' { * The recommended place to share data between hooks */ hookData: HookData; + /** + * The recommended place to emit logs + */ + logger: { + log: (msg: string) => void; + warn: (msg: string) => void; + error: (msg: string) => void; + }; } /** From d3405b0d49e88f55028038deb6733b0cc8e2482e Mon Sep 17 00:00:00 2001 From: hossam-nasr Date: Mon, 25 Apr 2022 14:34:05 -0700 Subject: [PATCH 07/39] refactor --- src/WorkerChannel.ts | 31 +++++++++++++++++-------------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/src/WorkerChannel.ts b/src/WorkerChannel.ts index b9b2869c..c1f7929c 100644 --- a/src/WorkerChannel.ts +++ b/src/WorkerChannel.ts @@ -130,7 +130,24 @@ export class WorkerChannel { async updateFunctionAppDirectory(functionAppDirectory: string): Promise { await this.#updatePackageJson(functionAppDirectory); + await this.#loadEntryPointFile(functionAppDirectory); + } + + async #updatePackageJson(dir: string): Promise { + try { + this.packageJson = await parsePackageJson(dir); + } catch (err) { + const error = ensureErrorType(err); + this.log({ + message: `Worker failed to load package.json: ${error.message}`, + level: LogLevel.Warning, + logCategory: LogCategory.System, + }); + this.packageJson = {}; + } + } + async #loadEntryPointFile(functionAppDirectory: string): Promise { const entryPointFile = this.packageJson.main; if (entryPointFile) { this.log({ @@ -158,18 +175,4 @@ export class WorkerChannel { } } } - - async #updatePackageJson(dir: string): Promise { - try { - this.packageJson = await parsePackageJson(dir); - } catch (err) { - const error = ensureErrorType(err); - this.log({ - message: `Worker failed to load package.json: ${error.message}`, - level: LogLevel.Warning, - logCategory: LogCategory.System, - }); - this.packageJson = {}; - } - } } From be07aee7bc5e39a7b570d29e425bab2b13df4c36 Mon Sep 17 00:00:00 2001 From: hossam-nasr Date: Mon, 25 Apr 2022 14:51:43 -0700 Subject: [PATCH 08/39] update comment --- types-core/index.d.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/types-core/index.d.ts b/types-core/index.d.ts index 7b64cce2..6ad96368 100644 --- a/types-core/index.d.ts +++ b/types-core/index.d.ts @@ -104,7 +104,7 @@ declare module '@azure/functions-core' { } /** - * Context on a function app that has is about to be deallocated + * Context on a function app that is about to be deallocated * This object will be passed to all app teardown hooks */ export interface AppTeardownContext extends HookContext { From 0e4502b4eaf7d87ce8706355714bb625af7a115a Mon Sep 17 00:00:00 2001 From: hossam-nasr Date: Tue, 26 Apr 2022 05:16:49 -0700 Subject: [PATCH 09/39] create copies of hook data for invocation contexts --- src/eventHandlers/InvocationHandler.ts | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src/eventHandlers/InvocationHandler.ts b/src/eventHandlers/InvocationHandler.ts index 19a18b32..b2cab9a3 100644 --- a/src/eventHandlers/InvocationHandler.ts +++ b/src/eventHandlers/InvocationHandler.ts @@ -2,7 +2,7 @@ // Licensed under the MIT License. import { AzureFunction } from '@azure/functions'; -import { PostInvocationContext, PreInvocationContext } from '@azure/functions-core'; +import { HookContext, PostInvocationContext, PreInvocationContext } from '@azure/functions-core'; import { format } from 'util'; import { AzureFunctionsRpcMessages as rpc } from '../../azure-functions-language-worker-protobuf/src/rpc'; import { CreateContextAndInputs } from '../Context'; @@ -91,8 +91,15 @@ export class InvocationHandler extends EventHandler<'invocationRequest', 'invoca }); let userFunction = channel.functionLoader.getFunc(functionId); - const preInvocContext: PreInvocationContext = { + + // create a copy of the hook data in the worker context (set by app hooks) + // the same hook data object is shared in the invocation hooks of the same invocation + const baseInvocationContext: HookContext = { ...channel.getBaseHookContext(), + }; + const preInvocContext: PreInvocationContext = { + hookData: baseInvocationContext.hookData, + logger: baseInvocationContext.logger, invocationContext: context, functionCallback: userFunction, inputs, @@ -116,7 +123,8 @@ export class InvocationHandler extends EventHandler<'invocationRequest', 'invoca } const postInvocContext: PostInvocationContext = { - ...channel.getBaseHookContext(), + hookData: baseInvocationContext.hookData, + logger: baseInvocationContext.logger, invocationContext: context, inputs, result: null, From aeec3ea642a23eb483ec3113c73164cf4cd270c4 Mon Sep 17 00:00:00 2001 From: hossam-nasr Date: Tue, 26 Apr 2022 05:20:14 -0700 Subject: [PATCH 10/39] don't copy for app startup hook data + add function app directory --- src/eventHandlers/WorkerInitHandler.ts | 12 +++++++----- types-core/index.d.ts | 4 ++-- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/src/eventHandlers/WorkerInitHandler.ts b/src/eventHandlers/WorkerInitHandler.ts index 1ec21a02..17d345bc 100644 --- a/src/eventHandlers/WorkerInitHandler.ts +++ b/src/eventHandlers/WorkerInitHandler.ts @@ -1,7 +1,7 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the MIT License. -import { AppStartupContext } from '@azure/functions-core'; +import { AppStartupContext, HookContext } from '@azure/functions-core'; import { access, constants } from 'fs'; import * as path from 'path'; import { AzureFunctionsRpcMessages as rpc } from '../../azure-functions-language-worker-protobuf/src/rpc'; @@ -36,11 +36,13 @@ export class WorkerInitHandler extends EventHandler<'workerInitRequest', 'worker await channel.updatePackageJson(functionAppDirectory); } - const context: AppStartupContext = { - ...channel.getBaseHookContext(), - something: '', + const baseContext: HookContext = channel.getBaseHookContext(); + const appStartupContext: AppStartupContext = { + logger: baseContext.logger, + hookData: baseContext.hookData, + functionAppDirectory: functionAppDirectory, }; - await channel.executeHooks('appStartup', context); + await channel.executeHooks('appStartup', appStartupContext); response.capabilities = { RpcHttpTriggerMetadataRemoved: 'true', diff --git a/types-core/index.d.ts b/types-core/index.d.ts index 6ad96368..71a1e3f0 100644 --- a/types-core/index.d.ts +++ b/types-core/index.d.ts @@ -100,7 +100,7 @@ declare module '@azure/functions-core' { * This object will be passed to all app startup hooks */ export interface AppStartupContext extends HookContext { - something: any; + functionAppDirectory: string; } /** @@ -108,7 +108,7 @@ declare module '@azure/functions-core' { * This object will be passed to all app teardown hooks */ export interface AppTeardownContext extends HookContext { - otherThing: any; + functionAppDirectory: string; } /** From e3f855cf9f7f3c89247b53024691e3314dc15475 Mon Sep 17 00:00:00 2001 From: hossam-nasr Date: Tue, 26 Apr 2022 06:50:13 -0700 Subject: [PATCH 11/39] function environment reload logic --- src/WorkerChannel.ts | 23 ++++++++++++++++++++--- src/eventHandlers/WorkerInitHandler.ts | 9 --------- 2 files changed, 20 insertions(+), 12 deletions(-) diff --git a/src/WorkerChannel.ts b/src/WorkerChannel.ts index c1f7929c..5d39cb11 100644 --- a/src/WorkerChannel.ts +++ b/src/WorkerChannel.ts @@ -1,7 +1,7 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the MIT License. -import { HookCallback, HookContext, HookData } from '@azure/functions-core'; +import { AppStartupContext, HookCallback, HookContext, HookData } from '@azure/functions-core'; import { pathExists } from 'fs-extra'; import { AzureFunctionsRpcMessages as rpc } from '../azure-functions-language-worker-protobuf/src/rpc'; import { Disposable } from './Disposable'; @@ -18,6 +18,7 @@ export class WorkerChannel { eventStream: IEventStream; functionLoader: IFunctionLoader; packageJson: PackageJson; + #functionAppDirectory = ''; #hookData: HookData = {}; #preInvocationHooks: HookCallback[] = []; #postInvocationHooks: HookCallback[] = []; @@ -129,8 +130,19 @@ export class WorkerChannel { } async updateFunctionAppDirectory(functionAppDirectory: string): Promise { - await this.#updatePackageJson(functionAppDirectory); - await this.#loadEntryPointFile(functionAppDirectory); + if (functionAppDirectory !== this.#functionAppDirectory) { + this.#functionAppDirectory = functionAppDirectory; + this.#clearHooks(); + await this.#updatePackageJson(functionAppDirectory); + await this.#loadEntryPointFile(functionAppDirectory); + const baseContext: HookContext = this.getBaseHookContext(); + const appStartupContext: AppStartupContext = { + logger: baseContext.logger, + hookData: baseContext.hookData, + functionAppDirectory: functionAppDirectory, + }; + await this.executeHooks('appStartup', appStartupContext); + } } async #updatePackageJson(dir: string): Promise { @@ -175,4 +187,9 @@ export class WorkerChannel { } } } + + #clearHooks() { + this.#preInvocationHooks = this.#postInvocationHooks = this.#appStartupHooks = this.#appTeardownHooks = []; + this.#hookData = {}; + } } diff --git a/src/eventHandlers/WorkerInitHandler.ts b/src/eventHandlers/WorkerInitHandler.ts index 17d345bc..45ba230c 100644 --- a/src/eventHandlers/WorkerInitHandler.ts +++ b/src/eventHandlers/WorkerInitHandler.ts @@ -1,7 +1,6 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the MIT License. -import { AppStartupContext, HookContext } from '@azure/functions-core'; import { access, constants } from 'fs'; import * as path from 'path'; import { AzureFunctionsRpcMessages as rpc } from '../../azure-functions-language-worker-protobuf/src/rpc'; @@ -36,14 +35,6 @@ export class WorkerInitHandler extends EventHandler<'workerInitRequest', 'worker await channel.updatePackageJson(functionAppDirectory); } - const baseContext: HookContext = channel.getBaseHookContext(); - const appStartupContext: AppStartupContext = { - logger: baseContext.logger, - hookData: baseContext.hookData, - functionAppDirectory: functionAppDirectory, - }; - await channel.executeHooks('appStartup', appStartupContext); - response.capabilities = { RpcHttpTriggerMetadataRemoved: 'true', RpcHttpBodyOnly: 'true', From 43fb5a0ef61f68569c85f554b8817fc2fb4e177d Mon Sep 17 00:00:00 2001 From: hossam-nasr Date: Tue, 26 Apr 2022 07:03:04 -0700 Subject: [PATCH 12/39] implement logger interface in hook context --- src/WorkerChannel.ts | 42 ++++++++++++++++-------------------------- types-core/index.d.ts | 8 ++------ 2 files changed, 18 insertions(+), 32 deletions(-) diff --git a/src/WorkerChannel.ts b/src/WorkerChannel.ts index 5d39cb11..6c777c46 100644 --- a/src/WorkerChannel.ts +++ b/src/WorkerChannel.ts @@ -3,6 +3,7 @@ import { AppStartupContext, HookCallback, HookContext, HookData } from '@azure/functions-core'; import { pathExists } from 'fs-extra'; +import { format } from 'util'; import { AzureFunctionsRpcMessages as rpc } from '../azure-functions-language-worker-protobuf/src/rpc'; import { Disposable } from './Disposable'; import { IFunctionLoader } from './FunctionLoader'; @@ -83,37 +84,26 @@ export class WorkerChannel { } getBaseHookContext(): HookContext { + const logger = Object.assign((...args: any[]) => this.#userLog(LogLevel.Information, ...args), { + info: (...args: any[]) => this.#userLog(LogLevel.Information, ...args), + warn: (...args: any[]) => this.#userLog(LogLevel.Warning, ...args), + error: (...args: any[]) => this.#userLog(LogLevel.Error, ...args), + verbose: (...args: any[]) => this.#userLog(LogLevel.Trace, ...args), + }); return { hookData: this.#hookData, - logger: { - log: (msg: string) => { - this.log({ - category: 'executionHooksLog', - message: msg, - level: LogLevel.Debug, - logCategory: LogCategory.User, - }); - }, - warn: (msg: string) => { - this.log({ - category: 'executionHooksWarn', - message: msg, - level: LogLevel.Warning, - logCategory: LogCategory.User, - }); - }, - error: (msg: string) => { - this.log({ - category: 'executionHooksError', - message: msg, - level: LogLevel.Error, - logCategory: LogCategory.User, - }); - }, - }, + logger, }; } + #userLog(level: LogLevel, ...args: any[]): void { + this.log({ + message: format.apply(null, <[any, any[]]>args), + logCategory: LogCategory.User, + level, + }); + } + #getHooks(hookName: string): HookCallback[] { switch (hookName) { case 'preInvocation': diff --git a/types-core/index.d.ts b/types-core/index.d.ts index 71a1e3f0..8150d9a4 100644 --- a/types-core/index.d.ts +++ b/types-core/index.d.ts @@ -1,7 +1,7 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the MIT License. -import { AzureFunction, Context } from '@azure/functions'; +import { AzureFunction, Context, Logger } from '@azure/functions'; /** * This module is shipped as a built-in part of the Azure Functions Node.js worker and is available at runtime @@ -41,11 +41,7 @@ declare module '@azure/functions-core' { /** * The recommended place to emit logs */ - logger: { - log: (msg: string) => void; - warn: (msg: string) => void; - error: (msg: string) => void; - }; + logger: Logger; } /** From 477c506576ae5158718b29c82a2f0360d7d084d4 Mon Sep 17 00:00:00 2001 From: hossam-nasr Date: Tue, 26 Apr 2022 07:10:08 -0700 Subject: [PATCH 13/39] add host version to startup hooks --- src/WorkerChannel.ts | 2 ++ src/eventHandlers/WorkerInitHandler.ts | 8 ++++---- types-core/index.d.ts | 10 ++++++++++ 3 files changed, 16 insertions(+), 4 deletions(-) diff --git a/src/WorkerChannel.ts b/src/WorkerChannel.ts index 6c777c46..44a63089 100644 --- a/src/WorkerChannel.ts +++ b/src/WorkerChannel.ts @@ -19,6 +19,7 @@ export class WorkerChannel { eventStream: IEventStream; functionLoader: IFunctionLoader; packageJson: PackageJson; + hostVersion = ''; #functionAppDirectory = ''; #hookData: HookData = {}; #preInvocationHooks: HookCallback[] = []; @@ -130,6 +131,7 @@ export class WorkerChannel { logger: baseContext.logger, hookData: baseContext.hookData, functionAppDirectory: functionAppDirectory, + hostVersion: this.hostVersion, }; await this.executeHooks('appStartup', appStartupContext); } diff --git a/src/eventHandlers/WorkerInitHandler.ts b/src/eventHandlers/WorkerInitHandler.ts index 45ba230c..c8e7e6c8 100644 --- a/src/eventHandlers/WorkerInitHandler.ts +++ b/src/eventHandlers/WorkerInitHandler.ts @@ -30,10 +30,10 @@ export class WorkerInitHandler extends EventHandler<'workerInitRequest', 'worker }); logColdStartWarning(channel); - const functionAppDirectory = msg.functionAppDirectory; - if (functionAppDirectory) { - await channel.updatePackageJson(functionAppDirectory); - } + const functionAppDirectory = nonNullProp(msg, 'functionAppDirectory'); + const hostVersion = nonNullProp(msg, 'hostVersion'); + channel.hostVersion = hostVersion; + await channel.updateFunctionAppDirectory(functionAppDirectory); response.capabilities = { RpcHttpTriggerMetadataRemoved: 'true', diff --git a/types-core/index.d.ts b/types-core/index.d.ts index 8150d9a4..3a0796c2 100644 --- a/types-core/index.d.ts +++ b/types-core/index.d.ts @@ -96,7 +96,14 @@ declare module '@azure/functions-core' { * This object will be passed to all app startup hooks */ export interface AppStartupContext extends HookContext { + /** + * Absolute directory of the function app + */ functionAppDirectory: string; + /** + * The version of the host running the function app + */ + hostVersion: string; } /** @@ -104,6 +111,9 @@ declare module '@azure/functions-core' { * This object will be passed to all app teardown hooks */ export interface AppTeardownContext extends HookContext { + /** + * Absolute directory of the function app + */ functionAppDirectory: string; } From d54b6cd96cb057fcb5b103b63a74c37ca922f802 Mon Sep 17 00:00:00 2001 From: hossam-nasr Date: Tue, 26 Apr 2022 07:32:57 -0700 Subject: [PATCH 14/39] fix tests --- src/WorkerChannel.ts | 8 ++++---- src/eventHandlers/WorkerInitHandler.ts | 5 +++-- test/eventHandlers/WorkerInitHandler.test.ts | 1 + types-core/index.d.ts | 2 +- 4 files changed, 9 insertions(+), 7 deletions(-) diff --git a/src/WorkerChannel.ts b/src/WorkerChannel.ts index 44a63089..743e5c82 100644 --- a/src/WorkerChannel.ts +++ b/src/WorkerChannel.ts @@ -19,8 +19,8 @@ export class WorkerChannel { eventStream: IEventStream; functionLoader: IFunctionLoader; packageJson: PackageJson; - hostVersion = ''; - #functionAppDirectory = ''; + hostVersion: string | undefined; + functionAppDirectory = ''; #hookData: HookData = {}; #preInvocationHooks: HookCallback[] = []; #postInvocationHooks: HookCallback[] = []; @@ -121,8 +121,8 @@ export class WorkerChannel { } async updateFunctionAppDirectory(functionAppDirectory: string): Promise { - if (functionAppDirectory !== this.#functionAppDirectory) { - this.#functionAppDirectory = functionAppDirectory; + if (functionAppDirectory !== this.functionAppDirectory) { + this.functionAppDirectory = functionAppDirectory; this.#clearHooks(); await this.#updatePackageJson(functionAppDirectory); await this.#loadEntryPointFile(functionAppDirectory); diff --git a/src/eventHandlers/WorkerInitHandler.ts b/src/eventHandlers/WorkerInitHandler.ts index c8e7e6c8..a71c930f 100644 --- a/src/eventHandlers/WorkerInitHandler.ts +++ b/src/eventHandlers/WorkerInitHandler.ts @@ -31,8 +31,9 @@ export class WorkerInitHandler extends EventHandler<'workerInitRequest', 'worker logColdStartWarning(channel); const functionAppDirectory = nonNullProp(msg, 'functionAppDirectory'); - const hostVersion = nonNullProp(msg, 'hostVersion'); - channel.hostVersion = hostVersion; + if (msg.hostVersion) { + channel.hostVersion = msg.hostVersion; + } await channel.updateFunctionAppDirectory(functionAppDirectory); response.capabilities = { diff --git a/test/eventHandlers/WorkerInitHandler.test.ts b/test/eventHandlers/WorkerInitHandler.test.ts index 0470bc99..67ff3344 100644 --- a/test/eventHandlers/WorkerInitHandler.test.ts +++ b/test/eventHandlers/WorkerInitHandler.test.ts @@ -131,6 +131,7 @@ describe('WorkerInitHandler', () => { afterEach(async () => { mockFs.restore(); await stream.afterEachEventHandlerTest(); + channel.functionAppDirectory = ''; }); it('responds to init', async () => { diff --git a/types-core/index.d.ts b/types-core/index.d.ts index 3a0796c2..99617b3d 100644 --- a/types-core/index.d.ts +++ b/types-core/index.d.ts @@ -103,7 +103,7 @@ declare module '@azure/functions-core' { /** * The version of the host running the function app */ - hostVersion: string; + hostVersion?: string; } /** From 7411dc5392c4f7e8dfd420e2f672f3539389973d Mon Sep 17 00:00:00 2001 From: hossam-nasr Date: Wed, 1 Jun 2022 17:14:54 -0700 Subject: [PATCH 15/39] rename app startup function --- src/WorkerChannel.ts | 32 +++++++------------ .../FunctionEnvironmentReloadHandler.ts | 3 +- src/eventHandlers/WorkerInitHandler.ts | 3 +- 3 files changed, 15 insertions(+), 23 deletions(-) diff --git a/src/WorkerChannel.ts b/src/WorkerChannel.ts index 743e5c82..16a4a6aa 100644 --- a/src/WorkerChannel.ts +++ b/src/WorkerChannel.ts @@ -20,7 +20,6 @@ export class WorkerChannel { functionLoader: IFunctionLoader; packageJson: PackageJson; hostVersion: string | undefined; - functionAppDirectory = ''; #hookData: HookData = {}; #preInvocationHooks: HookCallback[] = []; #postInvocationHooks: HookCallback[] = []; @@ -120,21 +119,17 @@ export class WorkerChannel { } } - async updateFunctionAppDirectory(functionAppDirectory: string): Promise { - if (functionAppDirectory !== this.functionAppDirectory) { - this.functionAppDirectory = functionAppDirectory; - this.#clearHooks(); - await this.#updatePackageJson(functionAppDirectory); - await this.#loadEntryPointFile(functionAppDirectory); - const baseContext: HookContext = this.getBaseHookContext(); - const appStartupContext: AppStartupContext = { - logger: baseContext.logger, - hookData: baseContext.hookData, - functionAppDirectory: functionAppDirectory, - hostVersion: this.hostVersion, - }; - await this.executeHooks('appStartup', appStartupContext); - } + async initalizeApp(functionAppDirectory: string): Promise { + await this.#updatePackageJson(functionAppDirectory); + await this.#loadEntryPointFile(functionAppDirectory); + const { logger, hookData }: HookContext = this.getBaseHookContext(); + const appStartupContext: AppStartupContext = { + logger, + hookData, + functionAppDirectory, + hostVersion: this.hostVersion, + }; + await this.executeHooks('appStartup', appStartupContext); } async #updatePackageJson(dir: string): Promise { @@ -179,9 +174,4 @@ export class WorkerChannel { } } } - - #clearHooks() { - this.#preInvocationHooks = this.#postInvocationHooks = this.#appStartupHooks = this.#appTeardownHooks = []; - this.#hookData = {}; - } } diff --git a/src/eventHandlers/FunctionEnvironmentReloadHandler.ts b/src/eventHandlers/FunctionEnvironmentReloadHandler.ts index 7a4ca37e..f1e4245b 100644 --- a/src/eventHandlers/FunctionEnvironmentReloadHandler.ts +++ b/src/eventHandlers/FunctionEnvironmentReloadHandler.ts @@ -35,6 +35,7 @@ export class FunctionEnvironmentReloadHandler extends EventHandler< }); process.env = Object.assign({}, msg.environmentVariables); + // Change current working directory if (msg.functionAppDirectory) { channel.log({ @@ -43,7 +44,7 @@ export class FunctionEnvironmentReloadHandler extends EventHandler< logCategory: LogCategory.System, }); process.chdir(msg.functionAppDirectory); - await channel.updateFunctionAppDirectory(msg.functionAppDirectory); + await channel.initalizeApp(msg.functionAppDirectory); } return response; diff --git a/src/eventHandlers/WorkerInitHandler.ts b/src/eventHandlers/WorkerInitHandler.ts index a71c930f..bc19551c 100644 --- a/src/eventHandlers/WorkerInitHandler.ts +++ b/src/eventHandlers/WorkerInitHandler.ts @@ -34,7 +34,8 @@ export class WorkerInitHandler extends EventHandler<'workerInitRequest', 'worker if (msg.hostVersion) { channel.hostVersion = msg.hostVersion; } - await channel.updateFunctionAppDirectory(functionAppDirectory); + + await channel.initalizeApp(functionAppDirectory); response.capabilities = { RpcHttpTriggerMetadataRemoved: 'true', From 651d34cf9bb76d7e2eec31e8cc3354f6c50ea376 Mon Sep 17 00:00:00 2001 From: hossam-nasr Date: Fri, 3 Jun 2022 11:04:38 -0700 Subject: [PATCH 16/39] remove teardown hooks --- src/WorkerChannel.ts | 3 --- types-core/index.d.ts | 13 ------------- 2 files changed, 16 deletions(-) diff --git a/src/WorkerChannel.ts b/src/WorkerChannel.ts index 16a4a6aa..32046e43 100644 --- a/src/WorkerChannel.ts +++ b/src/WorkerChannel.ts @@ -24,7 +24,6 @@ export class WorkerChannel { #preInvocationHooks: HookCallback[] = []; #postInvocationHooks: HookCallback[] = []; #appStartupHooks: HookCallback[] = []; - #appTeardownHooks: HookCallback[] = []; constructor(eventStream: IEventStream, functionLoader: IFunctionLoader) { this.eventStream = eventStream; @@ -112,8 +111,6 @@ export class WorkerChannel { return this.#postInvocationHooks; case 'appStartup': return this.#appStartupHooks; - case 'appTeardown': - return this.#appTeardownHooks; default: throw new RangeError(`Unrecognized hook "${hookName}"`); } diff --git a/types-core/index.d.ts b/types-core/index.d.ts index 99617b3d..52b3efad 100644 --- a/types-core/index.d.ts +++ b/types-core/index.d.ts @@ -19,14 +19,12 @@ declare module '@azure/functions-core' { export function registerHook(hookName: 'preInvocation', callback: PreInvocationCallback): Disposable; export function registerHook(hookName: 'postInvocation', callback: PostInvocationCallback): Disposable; export function registerHook(hookName: 'appStartup', callback: AppStartupCallback): Disposable; - export function registerHook(hookName: 'appTeardown', callback: AppTeardownCallback): Disposable; export function registerHook(hookName: string, callback: HookCallback): Disposable; export type HookCallback = (context: HookContext) => void | Promise; export type PreInvocationCallback = (context: PreInvocationContext) => void | Promise; export type PostInvocationCallback = (context: PostInvocationContext) => void | Promise; export type AppStartupCallback = (context: AppStartupContext) => void | Promise; - export type AppTeardownCallback = (context: AppTeardownCallback) => void | Promise; export type HookData = { [key: string]: any }; @@ -106,17 +104,6 @@ declare module '@azure/functions-core' { hostVersion?: string; } - /** - * Context on a function app that is about to be deallocated - * This object will be passed to all app teardown hooks - */ - export interface AppTeardownContext extends HookContext { - /** - * Absolute directory of the function app - */ - functionAppDirectory: string; - } - /** * Represents a type which can release resources, such as event listening or a timer. */ From 255fb5cbd9e9694b7fad875b277e40a4fe4bfbb6 Mon Sep 17 00:00:00 2001 From: hossam-nasr Date: Fri, 3 Jun 2022 11:18:40 -0700 Subject: [PATCH 17/39] proper message category and functionInvocationId in hook logs --- src/WorkerChannel.ts | 20 +++++++++++++------- src/eventHandlers/InvocationHandler.ts | 10 +++++----- 2 files changed, 18 insertions(+), 12 deletions(-) diff --git a/src/WorkerChannel.ts b/src/WorkerChannel.ts index 32046e43..ca51ba60 100644 --- a/src/WorkerChannel.ts +++ b/src/WorkerChannel.ts @@ -82,22 +82,28 @@ export class WorkerChannel { } } - getBaseHookContext(): HookContext { - const logger = Object.assign((...args: any[]) => this.#userLog(LogLevel.Information, ...args), { - info: (...args: any[]) => this.#userLog(LogLevel.Information, ...args), - warn: (...args: any[]) => this.#userLog(LogLevel.Warning, ...args), - error: (...args: any[]) => this.#userLog(LogLevel.Error, ...args), - verbose: (...args: any[]) => this.#userLog(LogLevel.Trace, ...args), + getBaseHookContext(functionInvocationId?: string, msgCategory?: string): HookContext { + const log = (logLevel: LogLevel, ...args: any[]) => + this.#userLog(logLevel, functionInvocationId, msgCategory, ...args); + + const logger = Object.assign((...args: any[]) => log(LogLevel.Information, ...args), { + info: (...args: any[]) => log(LogLevel.Information, ...args), + warn: (...args: any[]) => log(LogLevel.Warning, ...args), + error: (...args: any[]) => log(LogLevel.Error, ...args), + verbose: (...args: any[]) => log(LogLevel.Trace, ...args), }); + return { hookData: this.#hookData, logger, }; } - #userLog(level: LogLevel, ...args: any[]): void { + #userLog(level: LogLevel, functionInvocationId?: string, msgCategory?: string, ...args: any[]): void { this.log({ message: format.apply(null, <[any, any[]]>args), + category: msgCategory, + invocationId: functionInvocationId, logCategory: LogCategory.User, level, }); diff --git a/src/eventHandlers/InvocationHandler.ts b/src/eventHandlers/InvocationHandler.ts index b2cab9a3..968cc7c9 100644 --- a/src/eventHandlers/InvocationHandler.ts +++ b/src/eventHandlers/InvocationHandler.ts @@ -95,8 +95,9 @@ export class InvocationHandler extends EventHandler<'invocationRequest', 'invoca // create a copy of the hook data in the worker context (set by app hooks) // the same hook data object is shared in the invocation hooks of the same invocation const baseInvocationContext: HookContext = { - ...channel.getBaseHookContext(), + ...channel.getBaseHookContext(invocationId, msgCategory), }; + const preInvocContext: PreInvocationContext = { hookData: baseInvocationContext.hookData, logger: baseInvocationContext.logger, @@ -104,8 +105,7 @@ export class InvocationHandler extends EventHandler<'invocationRequest', 'invoca functionCallback: userFunction, inputs, }; - - await channel.executeHooks('preInvocation', preInvocContext, msg.invocationId, msgCategory); + await channel.executeHooks('preInvocation', preInvocContext, invocationId, msgCategory); inputs = preInvocContext.inputs; userFunction = preInvocContext.functionCallback; @@ -123,8 +123,8 @@ export class InvocationHandler extends EventHandler<'invocationRequest', 'invoca } const postInvocContext: PostInvocationContext = { - hookData: baseInvocationContext.hookData, - logger: baseInvocationContext.logger, + hookData: preInvocContext.hookData, + logger: preInvocContext.logger, invocationContext: context, inputs, result: null, From 30a773f14dbd7bebccae53c692aabb6ea3bffbca Mon Sep 17 00:00:00 2001 From: hossam-nasr Date: Fri, 3 Jun 2022 13:12:27 -0700 Subject: [PATCH 18/39] make sure it compiles --- test/eventHandlers/WorkerInitHandler.test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/test/eventHandlers/WorkerInitHandler.test.ts b/test/eventHandlers/WorkerInitHandler.test.ts index 67ff3344..0470bc99 100644 --- a/test/eventHandlers/WorkerInitHandler.test.ts +++ b/test/eventHandlers/WorkerInitHandler.test.ts @@ -131,7 +131,6 @@ describe('WorkerInitHandler', () => { afterEach(async () => { mockFs.restore(); await stream.afterEachEventHandlerTest(); - channel.functionAppDirectory = ''; }); it('responds to init', async () => { From b012b0f1623e7d49397aca823861f0e9d80378bf Mon Sep 17 00:00:00 2001 From: hossam-nasr Date: Wed, 8 Jun 2022 12:17:55 -0700 Subject: [PATCH 19/39] remove logger --- src/WorkerChannel.ts | 27 ++------------------------ src/eventHandlers/InvocationHandler.ts | 4 +--- types-core/index.d.ts | 6 +----- 3 files changed, 4 insertions(+), 33 deletions(-) diff --git a/src/WorkerChannel.ts b/src/WorkerChannel.ts index ca51ba60..866f8125 100644 --- a/src/WorkerChannel.ts +++ b/src/WorkerChannel.ts @@ -3,7 +3,6 @@ import { AppStartupContext, HookCallback, HookContext, HookData } from '@azure/functions-core'; import { pathExists } from 'fs-extra'; -import { format } from 'util'; import { AzureFunctionsRpcMessages as rpc } from '../azure-functions-language-worker-protobuf/src/rpc'; import { Disposable } from './Disposable'; import { IFunctionLoader } from './FunctionLoader'; @@ -82,33 +81,12 @@ export class WorkerChannel { } } - getBaseHookContext(functionInvocationId?: string, msgCategory?: string): HookContext { - const log = (logLevel: LogLevel, ...args: any[]) => - this.#userLog(logLevel, functionInvocationId, msgCategory, ...args); - - const logger = Object.assign((...args: any[]) => log(LogLevel.Information, ...args), { - info: (...args: any[]) => log(LogLevel.Information, ...args), - warn: (...args: any[]) => log(LogLevel.Warning, ...args), - error: (...args: any[]) => log(LogLevel.Error, ...args), - verbose: (...args: any[]) => log(LogLevel.Trace, ...args), - }); - + getBaseHookContext(): HookContext { return { hookData: this.#hookData, - logger, }; } - #userLog(level: LogLevel, functionInvocationId?: string, msgCategory?: string, ...args: any[]): void { - this.log({ - message: format.apply(null, <[any, any[]]>args), - category: msgCategory, - invocationId: functionInvocationId, - logCategory: LogCategory.User, - level, - }); - } - #getHooks(hookName: string): HookCallback[] { switch (hookName) { case 'preInvocation': @@ -125,9 +103,8 @@ export class WorkerChannel { async initalizeApp(functionAppDirectory: string): Promise { await this.#updatePackageJson(functionAppDirectory); await this.#loadEntryPointFile(functionAppDirectory); - const { logger, hookData }: HookContext = this.getBaseHookContext(); + const { hookData }: HookContext = this.getBaseHookContext(); const appStartupContext: AppStartupContext = { - logger, hookData, functionAppDirectory, hostVersion: this.hostVersion, diff --git a/src/eventHandlers/InvocationHandler.ts b/src/eventHandlers/InvocationHandler.ts index 968cc7c9..5e574153 100644 --- a/src/eventHandlers/InvocationHandler.ts +++ b/src/eventHandlers/InvocationHandler.ts @@ -95,12 +95,11 @@ export class InvocationHandler extends EventHandler<'invocationRequest', 'invoca // create a copy of the hook data in the worker context (set by app hooks) // the same hook data object is shared in the invocation hooks of the same invocation const baseInvocationContext: HookContext = { - ...channel.getBaseHookContext(invocationId, msgCategory), + ...channel.getBaseHookContext(), }; const preInvocContext: PreInvocationContext = { hookData: baseInvocationContext.hookData, - logger: baseInvocationContext.logger, invocationContext: context, functionCallback: userFunction, inputs, @@ -124,7 +123,6 @@ export class InvocationHandler extends EventHandler<'invocationRequest', 'invoca const postInvocContext: PostInvocationContext = { hookData: preInvocContext.hookData, - logger: preInvocContext.logger, invocationContext: context, inputs, result: null, diff --git a/types-core/index.d.ts b/types-core/index.d.ts index 52b3efad..f5f4c701 100644 --- a/types-core/index.d.ts +++ b/types-core/index.d.ts @@ -1,7 +1,7 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the MIT License. -import { AzureFunction, Context, Logger } from '@azure/functions'; +import { AzureFunction, Context } from '@azure/functions'; /** * This module is shipped as a built-in part of the Azure Functions Node.js worker and is available at runtime @@ -36,10 +36,6 @@ declare module '@azure/functions-core' { * The recommended place to share data between hooks */ hookData: HookData; - /** - * The recommended place to emit logs - */ - logger: Logger; } /** From 497da2b10f8f5787b10186ba83bf8b51e1dc8ddd Mon Sep 17 00:00:00 2001 From: hossam-nasr Date: Wed, 8 Jun 2022 12:37:20 -0700 Subject: [PATCH 20/39] refactor app startup code --- src/WorkerChannel.ts | 54 ++----------------- src/appStartup.ts | 53 ++++++++++++++++++ .../FunctionEnvironmentReloadHandler.ts | 3 +- src/eventHandlers/WorkerInitHandler.ts | 3 +- 4 files changed, 62 insertions(+), 51 deletions(-) create mode 100644 src/appStartup.ts diff --git a/src/WorkerChannel.ts b/src/WorkerChannel.ts index 866f8125..b876b303 100644 --- a/src/WorkerChannel.ts +++ b/src/WorkerChannel.ts @@ -1,16 +1,13 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the MIT License. -import { AppStartupContext, HookCallback, HookContext, HookData } from '@azure/functions-core'; -import { pathExists } from 'fs-extra'; +import { HookCallback, HookContext, HookData } from '@azure/functions-core'; import { AzureFunctionsRpcMessages as rpc } from '../azure-functions-language-worker-protobuf/src/rpc'; import { Disposable } from './Disposable'; import { IFunctionLoader } from './FunctionLoader'; import { IEventStream } from './GrpcClient'; -import { loadScriptFile } from './loadScriptFile'; import { PackageJson, parsePackageJson } from './parsers/parsePackageJson'; import { ensureErrorType } from './utils/ensureErrorType'; -import path = require('path'); import LogLevel = rpc.RpcLog.Level; import LogCategory = rpc.RpcLog.RpcLogCategory; @@ -81,12 +78,6 @@ export class WorkerChannel { } } - getBaseHookContext(): HookContext { - return { - hookData: this.#hookData, - }; - } - #getHooks(hookName: string): HookCallback[] { switch (hookName) { case 'preInvocation': @@ -100,19 +91,13 @@ export class WorkerChannel { } } - async initalizeApp(functionAppDirectory: string): Promise { - await this.#updatePackageJson(functionAppDirectory); - await this.#loadEntryPointFile(functionAppDirectory); - const { hookData }: HookContext = this.getBaseHookContext(); - const appStartupContext: AppStartupContext = { - hookData, - functionAppDirectory, - hostVersion: this.hostVersion, + getBaseHookContext(): HookContext { + return { + hookData: this.#hookData, }; - await this.executeHooks('appStartup', appStartupContext); } - async #updatePackageJson(dir: string): Promise { + async updatePackageJson(dir: string): Promise { try { this.packageJson = await parsePackageJson(dir); } catch (err) { @@ -125,33 +110,4 @@ export class WorkerChannel { this.packageJson = {}; } } - - async #loadEntryPointFile(functionAppDirectory: string): Promise { - const entryPointFile = this.packageJson.main; - if (entryPointFile) { - this.log({ - message: `Loading entry point "${entryPointFile}"`, - level: LogLevel.Debug, - logCategory: LogCategory.System, - }); - try { - const entryPointFullPath = path.join(functionAppDirectory, entryPointFile); - if (!(await pathExists(entryPointFullPath))) { - throw new Error(`file does not exist`); - } - - await loadScriptFile(entryPointFullPath, this.packageJson); - this.log({ - message: `Loaded entry point "${entryPointFile}"`, - level: LogLevel.Debug, - logCategory: LogCategory.System, - }); - } catch (err) { - const error = ensureErrorType(err); - error.isAzureFunctionsInternalException = true; - error.message = `Worker was unable to load entry point "${entryPointFile}": ${error.message}`; - throw error; - } - } - } } diff --git a/src/appStartup.ts b/src/appStartup.ts new file mode 100644 index 00000000..9e3f13b0 --- /dev/null +++ b/src/appStartup.ts @@ -0,0 +1,53 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the MIT License. + +import { AppStartupContext, HookContext } from '@azure/functions-core'; +import { pathExists } from 'fs-extra'; +import { AzureFunctionsRpcMessages as rpc } from '../azure-functions-language-worker-protobuf/src/rpc'; +import { loadScriptFile } from './loadScriptFile'; +import { ensureErrorType } from './utils/ensureErrorType'; +import { WorkerChannel } from './WorkerChannel'; +import path = require('path'); +import LogLevel = rpc.RpcLog.Level; +import LogCategory = rpc.RpcLog.RpcLogCategory; + +export async function appStartup(functionAppDirectory: string, channel: WorkerChannel): Promise { + await channel.updatePackageJson(functionAppDirectory); + await loadEntryPointFile(functionAppDirectory, channel); + const { hookData }: HookContext = channel.getBaseHookContext(); + const appStartupContext: AppStartupContext = { + hookData, + functionAppDirectory, + hostVersion: channel.hostVersion, + }; + await channel.executeHooks('appStartup', appStartupContext); +} + +async function loadEntryPointFile(functionAppDirectory: string, channel: WorkerChannel): Promise { + const entryPointFile = channel.packageJson.main; + if (entryPointFile) { + channel.log({ + message: `Loading entry point "${entryPointFile}"`, + level: LogLevel.Debug, + logCategory: LogCategory.System, + }); + try { + const entryPointFullPath = path.join(functionAppDirectory, entryPointFile); + if (!(await pathExists(entryPointFullPath))) { + throw new Error(`file does not exist`); + } + + await loadScriptFile(entryPointFullPath, channel.packageJson); + channel.log({ + message: `Loaded entry point "${entryPointFile}"`, + level: LogLevel.Debug, + logCategory: LogCategory.System, + }); + } catch (err) { + const error = ensureErrorType(err); + error.isAzureFunctionsInternalException = true; + error.message = `Worker was unable to load entry point "${entryPointFile}": ${error.message}`; + throw error; + } + } +} diff --git a/src/eventHandlers/FunctionEnvironmentReloadHandler.ts b/src/eventHandlers/FunctionEnvironmentReloadHandler.ts index f1e4245b..594f171c 100644 --- a/src/eventHandlers/FunctionEnvironmentReloadHandler.ts +++ b/src/eventHandlers/FunctionEnvironmentReloadHandler.ts @@ -2,6 +2,7 @@ // Licensed under the MIT License. import { AzureFunctionsRpcMessages as rpc } from '../../azure-functions-language-worker-protobuf/src/rpc'; +import { appStartup } from '../appStartup'; import { WorkerChannel } from '../WorkerChannel'; import { EventHandler } from './EventHandler'; import LogCategory = rpc.RpcLog.RpcLogCategory; @@ -44,7 +45,7 @@ export class FunctionEnvironmentReloadHandler extends EventHandler< logCategory: LogCategory.System, }); process.chdir(msg.functionAppDirectory); - await channel.initalizeApp(msg.functionAppDirectory); + await appStartup(msg.functionAppDirectory, channel); } return response; diff --git a/src/eventHandlers/WorkerInitHandler.ts b/src/eventHandlers/WorkerInitHandler.ts index bc19551c..19ce4539 100644 --- a/src/eventHandlers/WorkerInitHandler.ts +++ b/src/eventHandlers/WorkerInitHandler.ts @@ -4,6 +4,7 @@ import { access, constants } from 'fs'; import * as path from 'path'; import { AzureFunctionsRpcMessages as rpc } from '../../azure-functions-language-worker-protobuf/src/rpc'; +import { appStartup } from '../appStartup'; import { isError } from '../utils/ensureErrorType'; import { WorkerChannel } from '../WorkerChannel'; import { EventHandler } from './EventHandler'; @@ -35,7 +36,7 @@ export class WorkerInitHandler extends EventHandler<'workerInitRequest', 'worker channel.hostVersion = msg.hostVersion; } - await channel.initalizeApp(functionAppDirectory); + await appStartup(functionAppDirectory, channel); response.capabilities = { RpcHttpTriggerMetadataRemoved: 'true', From 6093ed8b4d10c88c8366ceb00ee01fb7593f9e7b Mon Sep 17 00:00:00 2001 From: hossam-nasr Date: Wed, 8 Jun 2022 12:40:31 -0700 Subject: [PATCH 21/39] don't reset the hook data --- src/WorkerChannel.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/WorkerChannel.ts b/src/WorkerChannel.ts index b876b303..44aca8ea 100644 --- a/src/WorkerChannel.ts +++ b/src/WorkerChannel.ts @@ -64,7 +64,6 @@ export class WorkerChannel { invocationId, category: msgCategory, }); - context.hookData = this.#hookData; for (const callback of callbacks) { await callback(context); } From 8601eeb8ff75267ab2648eae1b7751b0496be459 Mon Sep 17 00:00:00 2001 From: hossam-nasr Date: Wed, 8 Jun 2022 12:49:41 -0700 Subject: [PATCH 22/39] simplify copying hook data --- src/WorkerChannel.ts | 8 +------- src/appStartup.ts | 5 ++--- src/eventHandlers/InvocationHandler.ts | 10 +++++----- 3 files changed, 8 insertions(+), 15 deletions(-) diff --git a/src/WorkerChannel.ts b/src/WorkerChannel.ts index 44aca8ea..2c56e4fc 100644 --- a/src/WorkerChannel.ts +++ b/src/WorkerChannel.ts @@ -16,7 +16,7 @@ export class WorkerChannel { functionLoader: IFunctionLoader; packageJson: PackageJson; hostVersion: string | undefined; - #hookData: HookData = {}; + appHookData: HookData = {}; #preInvocationHooks: HookCallback[] = []; #postInvocationHooks: HookCallback[] = []; #appStartupHooks: HookCallback[] = []; @@ -90,12 +90,6 @@ export class WorkerChannel { } } - getBaseHookContext(): HookContext { - return { - hookData: this.#hookData, - }; - } - async updatePackageJson(dir: string): Promise { try { this.packageJson = await parsePackageJson(dir); diff --git a/src/appStartup.ts b/src/appStartup.ts index 9e3f13b0..4a08f7f9 100644 --- a/src/appStartup.ts +++ b/src/appStartup.ts @@ -1,7 +1,7 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the MIT License. -import { AppStartupContext, HookContext } from '@azure/functions-core'; +import { AppStartupContext } from '@azure/functions-core'; import { pathExists } from 'fs-extra'; import { AzureFunctionsRpcMessages as rpc } from '../azure-functions-language-worker-protobuf/src/rpc'; import { loadScriptFile } from './loadScriptFile'; @@ -14,9 +14,8 @@ import LogCategory = rpc.RpcLog.RpcLogCategory; export async function appStartup(functionAppDirectory: string, channel: WorkerChannel): Promise { await channel.updatePackageJson(functionAppDirectory); await loadEntryPointFile(functionAppDirectory, channel); - const { hookData }: HookContext = channel.getBaseHookContext(); const appStartupContext: AppStartupContext = { - hookData, + hookData: channel.appHookData, functionAppDirectory, hostVersion: channel.hostVersion, }; diff --git a/src/eventHandlers/InvocationHandler.ts b/src/eventHandlers/InvocationHandler.ts index 5e574153..f4a4d0e7 100644 --- a/src/eventHandlers/InvocationHandler.ts +++ b/src/eventHandlers/InvocationHandler.ts @@ -2,7 +2,7 @@ // Licensed under the MIT License. import { AzureFunction } from '@azure/functions'; -import { HookContext, PostInvocationContext, PreInvocationContext } from '@azure/functions-core'; +import { HookData, PostInvocationContext, PreInvocationContext } from '@azure/functions-core'; import { format } from 'util'; import { AzureFunctionsRpcMessages as rpc } from '../../azure-functions-language-worker-protobuf/src/rpc'; import { CreateContextAndInputs } from '../Context'; @@ -94,12 +94,12 @@ export class InvocationHandler extends EventHandler<'invocationRequest', 'invoca // create a copy of the hook data in the worker context (set by app hooks) // the same hook data object is shared in the invocation hooks of the same invocation - const baseInvocationContext: HookContext = { - ...channel.getBaseHookContext(), + const invocationHookData: HookData = { + ...channel.appHookData, }; const preInvocContext: PreInvocationContext = { - hookData: baseInvocationContext.hookData, + hookData: invocationHookData, invocationContext: context, functionCallback: userFunction, inputs, @@ -122,7 +122,7 @@ export class InvocationHandler extends EventHandler<'invocationRequest', 'invoca } const postInvocContext: PostInvocationContext = { - hookData: preInvocContext.hookData, + hookData: invocationHookData, invocationContext: context, inputs, result: null, From 6755f265bcc6402244fd8934456f1e40e0365c4b Mon Sep 17 00:00:00 2001 From: hossam-nasr Date: Wed, 8 Jun 2022 15:50:54 -0700 Subject: [PATCH 23/39] add specialization test for environment reload handler --- .../FunctionEnvironmentReloadHandler.test.ts | 45 ++++++++++++++++++- test/eventHandlers/WorkerInitHandler.test.ts | 2 +- 2 files changed, 44 insertions(+), 3 deletions(-) diff --git a/test/eventHandlers/FunctionEnvironmentReloadHandler.test.ts b/test/eventHandlers/FunctionEnvironmentReloadHandler.test.ts index 01563d8e..20b18f11 100644 --- a/test/eventHandlers/FunctionEnvironmentReloadHandler.test.ts +++ b/test/eventHandlers/FunctionEnvironmentReloadHandler.test.ts @@ -8,6 +8,7 @@ import { AzureFunctionsRpcMessages as rpc } from '../../azure-functions-language import { WorkerChannel } from '../../src/WorkerChannel'; import { beforeEventHandlerSuite } from './beforeEventHandlerSuite'; import { TestEventStream } from './TestEventStream'; +import { Msg as WorkerInitMsg } from './WorkerInitHandler.test'; import path = require('path'); import LogCategory = rpc.RpcLog.RpcLogCategory; import LogLevel = rpc.RpcLog.Level; @@ -69,9 +70,11 @@ describe('FunctionEnvironmentReloadHandler', () => { let stream: TestEventStream; let channel: WorkerChannel; - // Reset `process.env` after this test suite so it doesn't affect other tests + // Reset `process.env` and process.cwd() after this test suite so it doesn't affect other tests let originalEnv: NodeJS.ProcessEnv; + let originalCwd: string; before(() => { + originalCwd = process.cwd(); originalEnv = process.env; ({ stream, channel } = beforeEventHandlerSuite()); }); @@ -82,6 +85,7 @@ describe('FunctionEnvironmentReloadHandler', () => { afterEach(async () => { mock.restore(); + process.chdir(originalCwd); await stream.afterEachEventHandlerTest(); }); @@ -229,6 +233,43 @@ describe('FunctionEnvironmentReloadHandler', () => { }); await stream.assertCalledWith(Msg.reloadEnvVarsLog(0), Msg.changingCwdLog(newDirAbsolute), Msg.reloadSuccess); expect(channel.packageJson).to.deep.equal(newPackageJson); - process.chdir(cwd); + }); + + it('correctly loads package.json in specialization scenario', async () => { + const cwd = process.cwd(); + const tempDir = 'temp'; + const appDir = 'app'; + const packageJson = { + type: 'module', + hello: 'world', + }; + + mock({ + [tempDir]: {}, + [appDir]: { + 'package.json': JSON.stringify(packageJson), + }, + }); + + stream.addTestMessage(WorkerInitMsg.init(path.join(cwd, tempDir))); + await stream.assertCalledWith( + WorkerInitMsg.receivedInitLog, + WorkerInitMsg.warning(`Worker failed to load package.json: file does not exist`), + WorkerInitMsg.response + ); + expect(channel.packageJson).to.be.empty; + + stream.addTestMessage({ + requestId: 'id', + functionEnvironmentReloadRequest: { + functionAppDirectory: path.join(cwd, appDir), + }, + }); + await stream.assertCalledWith( + Msg.reloadEnvVarsLog(0), + Msg.changingCwdLog(path.join(cwd, appDir)), + Msg.reloadSuccess + ); + expect(channel.packageJson).to.deep.equal(packageJson); }); }); diff --git a/test/eventHandlers/WorkerInitHandler.test.ts b/test/eventHandlers/WorkerInitHandler.test.ts index 0470bc99..7a96de77 100644 --- a/test/eventHandlers/WorkerInitHandler.test.ts +++ b/test/eventHandlers/WorkerInitHandler.test.ts @@ -15,7 +15,7 @@ import path = require('path'); import LogCategory = rpc.RpcLog.RpcLogCategory; import LogLevel = rpc.RpcLog.Level; -namespace Msg { +export namespace Msg { export function init(functionAppDirectory: string = __dirname): rpc.IStreamingMessage { return { requestId: 'id', From 1711e80f574e50daeb2a5d64c78ef48aeac5c233 Mon Sep 17 00:00:00 2001 From: hossam-nasr Date: Wed, 8 Jun 2022 16:17:55 -0700 Subject: [PATCH 24/39] add test for loading entry point in specialization scenario --- .../FunctionEnvironmentReloadHandler.test.ts | 40 +++++++++++++++++++ test/eventHandlers/WorkerInitHandler.test.ts | 5 +-- 2 files changed, 41 insertions(+), 4 deletions(-) diff --git a/test/eventHandlers/FunctionEnvironmentReloadHandler.test.ts b/test/eventHandlers/FunctionEnvironmentReloadHandler.test.ts index 20b18f11..d2acca90 100644 --- a/test/eventHandlers/FunctionEnvironmentReloadHandler.test.ts +++ b/test/eventHandlers/FunctionEnvironmentReloadHandler.test.ts @@ -272,4 +272,44 @@ describe('FunctionEnvironmentReloadHandler', () => { ); expect(channel.packageJson).to.deep.equal(packageJson); }); + + for (const extension of ['.js', '.mjs', '.cjs']) { + it(`Loads entry point (${extension}) in specialization scenario`, async () => { + const cwd = process.cwd(); + const tempDir = 'temp'; + const fileName = `entryPointFiles/doNothing${extension}`; + const expectedPackageJson = { + main: fileName, + }; + mock({ + [tempDir]: {}, + [__dirname]: { + 'package.json': JSON.stringify(expectedPackageJson), + // 'require' and 'mockFs' don't play well together so we need these files in both the mock and real file systems + entryPointFiles: mock.load(path.join(__dirname, 'entryPointFiles')), + }, + }); + + stream.addTestMessage(WorkerInitMsg.init(path.join(cwd, tempDir))); + await stream.assertCalledWith( + WorkerInitMsg.receivedInitLog, + WorkerInitMsg.warning('Worker failed to load package.json: file does not exist'), + WorkerInitMsg.response + ); + + stream.addTestMessage({ + requestId: 'id', + functionEnvironmentReloadRequest: { + functionAppDirectory: __dirname, + }, + }); + await stream.assertCalledWith( + Msg.reloadEnvVarsLog(0), + Msg.changingCwdLog(__dirname), + WorkerInitMsg.loadingEntryPoint(fileName), + WorkerInitMsg.loadedEntryPoint(fileName), + Msg.reloadSuccess + ); + }); + } }); diff --git a/test/eventHandlers/WorkerInitHandler.test.ts b/test/eventHandlers/WorkerInitHandler.test.ts index 7a96de77..b90abeec 100644 --- a/test/eventHandlers/WorkerInitHandler.test.ts +++ b/test/eventHandlers/WorkerInitHandler.test.ts @@ -214,10 +214,7 @@ describe('WorkerInitHandler', () => { }); for (const extension of ['.js', '.mjs', '.cjs']) { - it(`Loads entry point (${extension})`, async function (this: ITestCallbackContext) { - // Should be re-enabled after https://github.com/Azure/azure-functions-nodejs-worker/pull/577 - this.skip(); - + it(`Loads entry point (${extension}) in non-specialization scenario`, async () => { const fileName = `entryPointFiles/doNothing${extension}`; const expectedPackageJson = { main: fileName, From 6b38e56e809d46bd246d1cfe9b8cb75c1754d2ef Mon Sep 17 00:00:00 2001 From: hossam-nasr Date: Wed, 8 Jun 2022 16:54:19 -0700 Subject: [PATCH 25/39] add test for running app startup hook in non-specialization scenario --- test/appStartup.test.ts | 78 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 78 insertions(+) create mode 100644 test/appStartup.test.ts diff --git a/test/appStartup.test.ts b/test/appStartup.test.ts new file mode 100644 index 00000000..f5000ccc --- /dev/null +++ b/test/appStartup.test.ts @@ -0,0 +1,78 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the MIT License. + +import * as coreTypes from '@azure/functions-core'; +import { expect } from 'chai'; +import * as sinon from 'sinon'; +import { AzureFunctionsRpcMessages as rpc } from '../azure-functions-language-worker-protobuf/src/rpc'; +import { WorkerChannel } from '../src/WorkerChannel'; +import { beforeEventHandlerSuite } from './eventHandlers/beforeEventHandlerSuite'; +import { TestEventStream } from './eventHandlers/TestEventStream'; +import { Msg as WorkerInitMsg } from './eventHandlers/WorkerInitHandler.test'; +import LogCategory = rpc.RpcLog.RpcLogCategory; +import LogLevel = rpc.RpcLog.Level; + +namespace Msg { + export function executingHooksLog(count: number, hookName: string): rpc.IStreamingMessage { + return { + rpcLog: { + category: undefined, + invocationId: undefined, + message: `Executing ${count} "${hookName}" hooks`, + level: LogLevel.Debug, + logCategory: LogCategory.System, + }, + }; + } + export function executedHooksLog(hookName: string): rpc.IStreamingMessage { + return { + rpcLog: { + category: undefined, + invocationId: undefined, + message: `Executed "${hookName}" hooks`, + level: LogLevel.Debug, + logCategory: LogCategory.System, + }, + }; + } +} + +describe('appStartup', () => { + let channel: WorkerChannel; + let stream: TestEventStream; + let coreApi: typeof coreTypes; + let testDisposables: coreTypes.Disposable[] = []; + + before(async () => { + ({ stream, channel } = beforeEventHandlerSuite()); + coreApi = await import('@azure/functions-core'); + }); + + afterEach(async () => { + await stream.afterEachEventHandlerTest(); + coreApi.Disposable.from(...testDisposables).dispose(); + testDisposables = []; + }); + + it('runs app startup hooks in non-specialization scenario', async () => { + const startupFunc = sinon.spy(); + testDisposables.push(coreApi.registerHook('appStartup', startupFunc)); + + stream.addTestMessage(WorkerInitMsg.init()); + + await stream.assertCalledWith( + WorkerInitMsg.receivedInitLog, + WorkerInitMsg.warning('Worker failed to load package.json: file does not exist'), + Msg.executingHooksLog(1, 'appStartup'), + Msg.executedHooksLog('appStartup'), + WorkerInitMsg.response + ); + + expect(startupFunc.callCount).to.be.equal(1); + expect(channel.packageJson).to.be.empty; + }); + it('runs app startup hooks only once in specialiation scenario', () => {}); + it('persists hookData changes from app startup hooks in worker channel', () => {}); + it('passes app startup hookData changes to invocation hooks', () => {}); + it('does not persist invocation hooks hookData changes', () => {}); +}); From 174e5ab9ae74018cd69f58ea1dd815a79cf62e97 Mon Sep 17 00:00:00 2001 From: hossam-nasr Date: Wed, 8 Jun 2022 17:48:57 -0700 Subject: [PATCH 26/39] assert called with right context --- test/appStartup.test.ts | 11 ++++++++++- test/eventHandlers/WorkerInitHandler.test.ts | 3 ++- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/test/appStartup.test.ts b/test/appStartup.test.ts index f5000ccc..ae0febf7 100644 --- a/test/appStartup.test.ts +++ b/test/appStartup.test.ts @@ -55,10 +55,18 @@ describe('appStartup', () => { }); it('runs app startup hooks in non-specialization scenario', async () => { + const hostVersion = '2.7.0'; + const functionAppDirectory = __dirname; + const expectedStartupContext: coreTypes.AppStartupContext = { + functionAppDirectory, + hostVersion, + hookData: {}, + }; + const startupFunc = sinon.spy(); testDisposables.push(coreApi.registerHook('appStartup', startupFunc)); - stream.addTestMessage(WorkerInitMsg.init()); + stream.addTestMessage(WorkerInitMsg.init(functionAppDirectory, hostVersion)); await stream.assertCalledWith( WorkerInitMsg.receivedInitLog, @@ -69,6 +77,7 @@ describe('appStartup', () => { ); expect(startupFunc.callCount).to.be.equal(1); + expect(startupFunc.args[0][0]).to.deep.equal(expectedStartupContext); expect(channel.packageJson).to.be.empty; }); it('runs app startup hooks only once in specialiation scenario', () => {}); diff --git a/test/eventHandlers/WorkerInitHandler.test.ts b/test/eventHandlers/WorkerInitHandler.test.ts index b90abeec..002a248a 100644 --- a/test/eventHandlers/WorkerInitHandler.test.ts +++ b/test/eventHandlers/WorkerInitHandler.test.ts @@ -16,12 +16,13 @@ import LogCategory = rpc.RpcLog.RpcLogCategory; import LogLevel = rpc.RpcLog.Level; export namespace Msg { - export function init(functionAppDirectory: string = __dirname): rpc.IStreamingMessage { + export function init(functionAppDirectory: string = __dirname, hostVersion?: string): rpc.IStreamingMessage { return { requestId: 'id', workerInitRequest: { capabilities: {}, functionAppDirectory, + hostVersion, }, }; } From 797c9e5dfb3a3a993bcfa6278a49ddf457d60843 Mon Sep 17 00:00:00 2001 From: hossam-nasr Date: Wed, 8 Jun 2022 17:57:52 -0700 Subject: [PATCH 27/39] add test for specialization scenario --- test/appStartup.test.ts | 55 +++++++++++++++++-- .../FunctionEnvironmentReloadHandler.test.ts | 2 +- 2 files changed, 51 insertions(+), 6 deletions(-) diff --git a/test/appStartup.test.ts b/test/appStartup.test.ts index ae0febf7..6be2dc2a 100644 --- a/test/appStartup.test.ts +++ b/test/appStartup.test.ts @@ -5,8 +5,8 @@ import * as coreTypes from '@azure/functions-core'; import { expect } from 'chai'; import * as sinon from 'sinon'; import { AzureFunctionsRpcMessages as rpc } from '../azure-functions-language-worker-protobuf/src/rpc'; -import { WorkerChannel } from '../src/WorkerChannel'; import { beforeEventHandlerSuite } from './eventHandlers/beforeEventHandlerSuite'; +import { Msg as EnvReloadMsg } from './eventHandlers/FunctionEnvironmentReloadHandler.test'; import { TestEventStream } from './eventHandlers/TestEventStream'; import { Msg as WorkerInitMsg } from './eventHandlers/WorkerInitHandler.test'; import LogCategory = rpc.RpcLog.RpcLogCategory; @@ -38,20 +38,29 @@ namespace Msg { } describe('appStartup', () => { - let channel: WorkerChannel; + // let channel: WorkerChannel; let stream: TestEventStream; let coreApi: typeof coreTypes; let testDisposables: coreTypes.Disposable[] = []; + let originalEnv: NodeJS.ProcessEnv; + let originalCwd: string; before(async () => { - ({ stream, channel } = beforeEventHandlerSuite()); + originalCwd = process.cwd(); + originalEnv = process.env; + ({ stream } = beforeEventHandlerSuite()); coreApi = await import('@azure/functions-core'); }); + after(() => { + process.env = originalEnv; + }); + afterEach(async () => { await stream.afterEachEventHandlerTest(); coreApi.Disposable.from(...testDisposables).dispose(); testDisposables = []; + process.chdir(originalCwd); }); it('runs app startup hooks in non-specialization scenario', async () => { @@ -78,9 +87,45 @@ describe('appStartup', () => { expect(startupFunc.callCount).to.be.equal(1); expect(startupFunc.args[0][0]).to.deep.equal(expectedStartupContext); - expect(channel.packageJson).to.be.empty; }); - it('runs app startup hooks only once in specialiation scenario', () => {}); + + it('runs app startup hooks only once in specialiation scenario', async () => { + const hostVersion = '2.7.0'; + const functionAppDirectory = __dirname; + const expectedStartupContext: coreTypes.AppStartupContext = { + functionAppDirectory, + hostVersion, + hookData: {}, + }; + const startupFunc = sinon.spy(); + + stream.addTestMessage(WorkerInitMsg.init(functionAppDirectory, hostVersion)); + await stream.assertCalledWith( + WorkerInitMsg.receivedInitLog, + WorkerInitMsg.warning('Worker failed to load package.json: file does not exist'), + WorkerInitMsg.response + ); + + testDisposables.push(coreApi.registerHook('appStartup', startupFunc)); + + stream.addTestMessage({ + requestId: 'id', + functionEnvironmentReloadRequest: { + functionAppDirectory, + }, + }); + await stream.assertCalledWith( + EnvReloadMsg.reloadEnvVarsLog(0), + EnvReloadMsg.changingCwdLog(functionAppDirectory), + WorkerInitMsg.warning('Worker failed to load package.json: file does not exist'), + Msg.executingHooksLog(1, 'appStartup'), + Msg.executedHooksLog('appStartup'), + EnvReloadMsg.reloadSuccess + ); + + expect(startupFunc.callCount).to.be.equal(1); + expect(startupFunc.args[0][0]).to.deep.equal(expectedStartupContext); + }); it('persists hookData changes from app startup hooks in worker channel', () => {}); it('passes app startup hookData changes to invocation hooks', () => {}); it('does not persist invocation hooks hookData changes', () => {}); diff --git a/test/eventHandlers/FunctionEnvironmentReloadHandler.test.ts b/test/eventHandlers/FunctionEnvironmentReloadHandler.test.ts index d2acca90..8c86f791 100644 --- a/test/eventHandlers/FunctionEnvironmentReloadHandler.test.ts +++ b/test/eventHandlers/FunctionEnvironmentReloadHandler.test.ts @@ -13,7 +13,7 @@ import path = require('path'); import LogCategory = rpc.RpcLog.RpcLogCategory; import LogLevel = rpc.RpcLog.Level; -namespace Msg { +export namespace Msg { export function reloadEnvVarsLog(numVars: number): rpc.IStreamingMessage { return { rpcLog: { From 1914121edb053d7683ec28249c990ef07306ea85 Mon Sep 17 00:00:00 2001 From: hossam-nasr Date: Wed, 8 Jun 2022 18:14:58 -0700 Subject: [PATCH 28/39] add test for persisting hook data from startup hooks --- src/appStartup.ts | 1 + test/appStartup.test.ts | 34 +++++++++++++++++++++++++++++++--- 2 files changed, 32 insertions(+), 3 deletions(-) diff --git a/src/appStartup.ts b/src/appStartup.ts index 4a08f7f9..2480a080 100644 --- a/src/appStartup.ts +++ b/src/appStartup.ts @@ -20,6 +20,7 @@ export async function appStartup(functionAppDirectory: string, channel: WorkerCh hostVersion: channel.hostVersion, }; await channel.executeHooks('appStartup', appStartupContext); + channel.appHookData = appStartupContext.hookData; } async function loadEntryPointFile(functionAppDirectory: string, channel: WorkerChannel): Promise { diff --git a/test/appStartup.test.ts b/test/appStartup.test.ts index 6be2dc2a..0115dc78 100644 --- a/test/appStartup.test.ts +++ b/test/appStartup.test.ts @@ -5,6 +5,7 @@ import * as coreTypes from '@azure/functions-core'; import { expect } from 'chai'; import * as sinon from 'sinon'; import { AzureFunctionsRpcMessages as rpc } from '../azure-functions-language-worker-protobuf/src/rpc'; +import { WorkerChannel } from '../src/WorkerChannel'; import { beforeEventHandlerSuite } from './eventHandlers/beforeEventHandlerSuite'; import { Msg as EnvReloadMsg } from './eventHandlers/FunctionEnvironmentReloadHandler.test'; import { TestEventStream } from './eventHandlers/TestEventStream'; @@ -38,7 +39,7 @@ namespace Msg { } describe('appStartup', () => { - // let channel: WorkerChannel; + let channel: WorkerChannel; let stream: TestEventStream; let coreApi: typeof coreTypes; let testDisposables: coreTypes.Disposable[] = []; @@ -48,7 +49,7 @@ describe('appStartup', () => { before(async () => { originalCwd = process.cwd(); originalEnv = process.env; - ({ stream } = beforeEventHandlerSuite()); + ({ stream, channel } = beforeEventHandlerSuite()); coreApi = await import('@azure/functions-core'); }); @@ -126,7 +127,34 @@ describe('appStartup', () => { expect(startupFunc.callCount).to.be.equal(1); expect(startupFunc.args[0][0]).to.deep.equal(expectedStartupContext); }); - it('persists hookData changes from app startup hooks in worker channel', () => {}); + + it('persists hookData changes from app startup hooks in worker channel', async () => { + const functionAppDirectory = __dirname; + const expectedHookData = { + hello: 'world', + test: { + test2: 3, + }, + }; + const startupFunc = sinon.spy((context: coreTypes.AppStartupContext) => { + context.hookData = expectedHookData; + }); + testDisposables.push(coreApi.registerHook('appStartup', startupFunc)); + + stream.addTestMessage(WorkerInitMsg.init(functionAppDirectory)); + + await stream.assertCalledWith( + WorkerInitMsg.receivedInitLog, + WorkerInitMsg.warning('Worker failed to load package.json: file does not exist'), + Msg.executingHooksLog(1, 'appStartup'), + Msg.executedHooksLog('appStartup'), + WorkerInitMsg.response + ); + + expect(startupFunc.callCount).to.be.equal(1); + expect(channel.appHookData).to.deep.equal(expectedHookData); + }); + it('passes app startup hookData changes to invocation hooks', () => {}); it('does not persist invocation hooks hookData changes', () => {}); }); From 164f85d86d1e321b78c790aeb5a81643c4966f44 Mon Sep 17 00:00:00 2001 From: hossam-nasr Date: Fri, 10 Jun 2022 13:24:09 -0700 Subject: [PATCH 29/39] add comment --- src/appStartup.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/appStartup.ts b/src/appStartup.ts index 2480a080..0f0b3c3b 100644 --- a/src/appStartup.ts +++ b/src/appStartup.ts @@ -11,6 +11,14 @@ import path = require('path'); import LogLevel = rpc.RpcLog.Level; import LogCategory = rpc.RpcLog.RpcLogCategory; +/** + * App startup can happen in two places, depending on if the worker was specialized or not + * 1. The worker can start in "normal" mode, meaning `workerInitRequest` will reference the user's app + * 2. The worker can start in "placeholder" mode, meaning `workerInitRequest` will reference a dummy app to "warm up" the worker and `functionEnvironmentReloadRequest` will be sent with the user's actual app. + * This process is called worker specialization and it helps with cold start times. + * The dummy app should never have actual startup code, so it should be safe to call `appStartup` twice in this case + * Worker specialization happens only once, so we don't need to worry about cleaning up resources from previous `functionEnvironmentReloadRequest`s. + */ export async function appStartup(functionAppDirectory: string, channel: WorkerChannel): Promise { await channel.updatePackageJson(functionAppDirectory); await loadEntryPointFile(functionAppDirectory, channel); From 5065c1e310055628810e16267166d084220672ae Mon Sep 17 00:00:00 2001 From: hossam-nasr Date: Thu, 23 Jun 2022 14:57:29 -0700 Subject: [PATCH 30/39] allow functionAppDirectory to be undefined in workerInitRequest --- src/eventHandlers/WorkerInitHandler.ts | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/eventHandlers/WorkerInitHandler.ts b/src/eventHandlers/WorkerInitHandler.ts index 19ce4539..6aa9d93b 100644 --- a/src/eventHandlers/WorkerInitHandler.ts +++ b/src/eventHandlers/WorkerInitHandler.ts @@ -31,12 +31,20 @@ export class WorkerInitHandler extends EventHandler<'workerInitRequest', 'worker }); logColdStartWarning(channel); - const functionAppDirectory = nonNullProp(msg, 'functionAppDirectory'); + if (msg.hostVersion) { channel.hostVersion = msg.hostVersion; } - await appStartup(functionAppDirectory, channel); + if (msg.functionAppDirectory) { + await appStartup(msg.functionAppDirectory, channel); + } else { + channel.log({ + message: 'functionAppDirectory was undefined in workerInitRequest message. Skipping startup logic.', + level: LogLevel.Warning, + logCategory: LogCategory.System, + }); + } response.capabilities = { RpcHttpTriggerMetadataRemoved: 'true', From 1747d5100aa06103a7eb01c7a40529d11a96407e Mon Sep 17 00:00:00 2001 From: hossam-nasr Date: Thu, 23 Jun 2022 15:09:56 -0700 Subject: [PATCH 31/39] rename to appStart --- src/WorkerChannel.ts | 6 +-- .../FunctionEnvironmentReloadHandler.ts | 4 +- src/eventHandlers/WorkerInitHandler.ts | 4 +- src/{appStartup.ts => startApp.ts} | 14 +++--- test/appStartup.test.ts | 48 +++++++++---------- types-core/index.d.ts | 8 ++-- 6 files changed, 42 insertions(+), 42 deletions(-) rename src/{appStartup.ts => startApp.ts} (84%) diff --git a/src/WorkerChannel.ts b/src/WorkerChannel.ts index 2c56e4fc..ecec0a20 100644 --- a/src/WorkerChannel.ts +++ b/src/WorkerChannel.ts @@ -19,7 +19,7 @@ export class WorkerChannel { appHookData: HookData = {}; #preInvocationHooks: HookCallback[] = []; #postInvocationHooks: HookCallback[] = []; - #appStartupHooks: HookCallback[] = []; + #appStartHooks: HookCallback[] = []; constructor(eventStream: IEventStream, functionLoader: IFunctionLoader) { this.eventStream = eventStream; @@ -83,8 +83,8 @@ export class WorkerChannel { return this.#preInvocationHooks; case 'postInvocation': return this.#postInvocationHooks; - case 'appStartup': - return this.#appStartupHooks; + case 'appStart': + return this.#appStartHooks; default: throw new RangeError(`Unrecognized hook "${hookName}"`); } diff --git a/src/eventHandlers/FunctionEnvironmentReloadHandler.ts b/src/eventHandlers/FunctionEnvironmentReloadHandler.ts index 594f171c..d28eab80 100644 --- a/src/eventHandlers/FunctionEnvironmentReloadHandler.ts +++ b/src/eventHandlers/FunctionEnvironmentReloadHandler.ts @@ -2,7 +2,7 @@ // Licensed under the MIT License. import { AzureFunctionsRpcMessages as rpc } from '../../azure-functions-language-worker-protobuf/src/rpc'; -import { appStartup } from '../appStartup'; +import { startApp } from '../startApp'; import { WorkerChannel } from '../WorkerChannel'; import { EventHandler } from './EventHandler'; import LogCategory = rpc.RpcLog.RpcLogCategory; @@ -45,7 +45,7 @@ export class FunctionEnvironmentReloadHandler extends EventHandler< logCategory: LogCategory.System, }); process.chdir(msg.functionAppDirectory); - await appStartup(msg.functionAppDirectory, channel); + await startApp(msg.functionAppDirectory, channel); } return response; diff --git a/src/eventHandlers/WorkerInitHandler.ts b/src/eventHandlers/WorkerInitHandler.ts index 6aa9d93b..0e6213d5 100644 --- a/src/eventHandlers/WorkerInitHandler.ts +++ b/src/eventHandlers/WorkerInitHandler.ts @@ -4,7 +4,7 @@ import { access, constants } from 'fs'; import * as path from 'path'; import { AzureFunctionsRpcMessages as rpc } from '../../azure-functions-language-worker-protobuf/src/rpc'; -import { appStartup } from '../appStartup'; +import { startApp } from '../startApp'; import { isError } from '../utils/ensureErrorType'; import { WorkerChannel } from '../WorkerChannel'; import { EventHandler } from './EventHandler'; @@ -37,7 +37,7 @@ export class WorkerInitHandler extends EventHandler<'workerInitRequest', 'worker } if (msg.functionAppDirectory) { - await appStartup(msg.functionAppDirectory, channel); + await startApp(msg.functionAppDirectory, channel); } else { channel.log({ message: 'functionAppDirectory was undefined in workerInitRequest message. Skipping startup logic.', diff --git a/src/appStartup.ts b/src/startApp.ts similarity index 84% rename from src/appStartup.ts rename to src/startApp.ts index 0f0b3c3b..2ccc67c1 100644 --- a/src/appStartup.ts +++ b/src/startApp.ts @@ -1,7 +1,7 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the MIT License. -import { AppStartupContext } from '@azure/functions-core'; +import { AppStartContext } from '@azure/functions-core'; import { pathExists } from 'fs-extra'; import { AzureFunctionsRpcMessages as rpc } from '../azure-functions-language-worker-protobuf/src/rpc'; import { loadScriptFile } from './loadScriptFile'; @@ -12,23 +12,23 @@ import LogLevel = rpc.RpcLog.Level; import LogCategory = rpc.RpcLog.RpcLogCategory; /** - * App startup can happen in two places, depending on if the worker was specialized or not + * Starting an app can happen in two places, depending on if the worker was specialized or not * 1. The worker can start in "normal" mode, meaning `workerInitRequest` will reference the user's app * 2. The worker can start in "placeholder" mode, meaning `workerInitRequest` will reference a dummy app to "warm up" the worker and `functionEnvironmentReloadRequest` will be sent with the user's actual app. * This process is called worker specialization and it helps with cold start times. - * The dummy app should never have actual startup code, so it should be safe to call `appStartup` twice in this case + * The dummy app should never have actual startup code, so it should be safe to call `startApp` twice in this case * Worker specialization happens only once, so we don't need to worry about cleaning up resources from previous `functionEnvironmentReloadRequest`s. */ -export async function appStartup(functionAppDirectory: string, channel: WorkerChannel): Promise { +export async function startApp(functionAppDirectory: string, channel: WorkerChannel): Promise { await channel.updatePackageJson(functionAppDirectory); await loadEntryPointFile(functionAppDirectory, channel); - const appStartupContext: AppStartupContext = { + const appStartContext: AppStartContext = { hookData: channel.appHookData, functionAppDirectory, hostVersion: channel.hostVersion, }; - await channel.executeHooks('appStartup', appStartupContext); - channel.appHookData = appStartupContext.hookData; + await channel.executeHooks('appStart', appStartContext); + channel.appHookData = appStartContext.hookData; } async function loadEntryPointFile(functionAppDirectory: string, channel: WorkerChannel): Promise { diff --git a/test/appStartup.test.ts b/test/appStartup.test.ts index 0115dc78..c5397d63 100644 --- a/test/appStartup.test.ts +++ b/test/appStartup.test.ts @@ -38,7 +38,7 @@ namespace Msg { } } -describe('appStartup', () => { +describe('startApp', () => { let channel: WorkerChannel; let stream: TestEventStream; let coreApi: typeof coreTypes; @@ -64,41 +64,41 @@ describe('appStartup', () => { process.chdir(originalCwd); }); - it('runs app startup hooks in non-specialization scenario', async () => { + it('runs app start hooks in non-specialization scenario', async () => { const hostVersion = '2.7.0'; const functionAppDirectory = __dirname; - const expectedStartupContext: coreTypes.AppStartupContext = { + const expectedStartContext: coreTypes.AppStartContext = { functionAppDirectory, hostVersion, hookData: {}, }; - const startupFunc = sinon.spy(); - testDisposables.push(coreApi.registerHook('appStartup', startupFunc)); + const startFunc = sinon.spy(); + testDisposables.push(coreApi.registerHook('appStart', startFunc)); stream.addTestMessage(WorkerInitMsg.init(functionAppDirectory, hostVersion)); await stream.assertCalledWith( WorkerInitMsg.receivedInitLog, WorkerInitMsg.warning('Worker failed to load package.json: file does not exist'), - Msg.executingHooksLog(1, 'appStartup'), - Msg.executedHooksLog('appStartup'), + Msg.executingHooksLog(1, 'appStart'), + Msg.executedHooksLog('appStart'), WorkerInitMsg.response ); - expect(startupFunc.callCount).to.be.equal(1); - expect(startupFunc.args[0][0]).to.deep.equal(expectedStartupContext); + expect(startFunc.callCount).to.be.equal(1); + expect(startFunc.args[0][0]).to.deep.equal(expectedStartContext); }); - it('runs app startup hooks only once in specialiation scenario', async () => { + it('runs app start hooks only once in specialiation scenario', async () => { const hostVersion = '2.7.0'; const functionAppDirectory = __dirname; - const expectedStartupContext: coreTypes.AppStartupContext = { + const expectedStartContext: coreTypes.AppStartContext = { functionAppDirectory, hostVersion, hookData: {}, }; - const startupFunc = sinon.spy(); + const startFunc = sinon.spy(); stream.addTestMessage(WorkerInitMsg.init(functionAppDirectory, hostVersion)); await stream.assertCalledWith( @@ -107,7 +107,7 @@ describe('appStartup', () => { WorkerInitMsg.response ); - testDisposables.push(coreApi.registerHook('appStartup', startupFunc)); + testDisposables.push(coreApi.registerHook('appStart', startFunc)); stream.addTestMessage({ requestId: 'id', @@ -119,16 +119,16 @@ describe('appStartup', () => { EnvReloadMsg.reloadEnvVarsLog(0), EnvReloadMsg.changingCwdLog(functionAppDirectory), WorkerInitMsg.warning('Worker failed to load package.json: file does not exist'), - Msg.executingHooksLog(1, 'appStartup'), - Msg.executedHooksLog('appStartup'), + Msg.executingHooksLog(1, 'appStart'), + Msg.executedHooksLog('appStart'), EnvReloadMsg.reloadSuccess ); - expect(startupFunc.callCount).to.be.equal(1); - expect(startupFunc.args[0][0]).to.deep.equal(expectedStartupContext); + expect(startFunc.callCount).to.be.equal(1); + expect(startFunc.args[0][0]).to.deep.equal(expectedStartContext); }); - it('persists hookData changes from app startup hooks in worker channel', async () => { + it('persists hookData changes from app start hooks in worker channel', async () => { const functionAppDirectory = __dirname; const expectedHookData = { hello: 'world', @@ -136,25 +136,25 @@ describe('appStartup', () => { test2: 3, }, }; - const startupFunc = sinon.spy((context: coreTypes.AppStartupContext) => { + const startFunc = sinon.spy((context: coreTypes.AppStartContext) => { context.hookData = expectedHookData; }); - testDisposables.push(coreApi.registerHook('appStartup', startupFunc)); + testDisposables.push(coreApi.registerHook('appStart', startFunc)); stream.addTestMessage(WorkerInitMsg.init(functionAppDirectory)); await stream.assertCalledWith( WorkerInitMsg.receivedInitLog, WorkerInitMsg.warning('Worker failed to load package.json: file does not exist'), - Msg.executingHooksLog(1, 'appStartup'), - Msg.executedHooksLog('appStartup'), + Msg.executingHooksLog(1, 'appStart'), + Msg.executedHooksLog('appStart'), WorkerInitMsg.response ); - expect(startupFunc.callCount).to.be.equal(1); + expect(startFunc.callCount).to.be.equal(1); expect(channel.appHookData).to.deep.equal(expectedHookData); }); - it('passes app startup hookData changes to invocation hooks', () => {}); + it('passes app start hookData changes to invocation hooks', () => {}); it('does not persist invocation hooks hookData changes', () => {}); }); diff --git a/types-core/index.d.ts b/types-core/index.d.ts index f5f4c701..8a4c6314 100644 --- a/types-core/index.d.ts +++ b/types-core/index.d.ts @@ -18,13 +18,13 @@ declare module '@azure/functions-core' { */ export function registerHook(hookName: 'preInvocation', callback: PreInvocationCallback): Disposable; export function registerHook(hookName: 'postInvocation', callback: PostInvocationCallback): Disposable; - export function registerHook(hookName: 'appStartup', callback: AppStartupCallback): Disposable; + export function registerHook(hookName: 'appStart', callback: AppStartCallback): Disposable; export function registerHook(hookName: string, callback: HookCallback): Disposable; export type HookCallback = (context: HookContext) => void | Promise; export type PreInvocationCallback = (context: PreInvocationContext) => void | Promise; export type PostInvocationCallback = (context: PostInvocationContext) => void | Promise; - export type AppStartupCallback = (context: AppStartupContext) => void | Promise; + export type AppStartCallback = (context: AppStartContext) => void | Promise; export type HookData = { [key: string]: any }; @@ -87,9 +87,9 @@ declare module '@azure/functions-core' { /** * Context on a function app that is about to be started - * This object will be passed to all app startup hooks + * This object will be passed to all app start hooks */ - export interface AppStartupContext extends HookContext { + export interface AppStartContext extends HookContext { /** * Absolute directory of the function app */ From 502e3e8c37119c593b96f1fef012e66dce3deeff Mon Sep 17 00:00:00 2001 From: hossam-nasr Date: Thu, 23 Jun 2022 16:14:42 -0700 Subject: [PATCH 32/39] one more file rename --- test/{appStartup.test.ts => startApp.test.ts} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename test/{appStartup.test.ts => startApp.test.ts} (100%) diff --git a/test/appStartup.test.ts b/test/startApp.test.ts similarity index 100% rename from test/appStartup.test.ts rename to test/startApp.test.ts From c273e61dd53aced166932fcc23b81d394b8ef672 Mon Sep 17 00:00:00 2001 From: hossam-nasr Date: Thu, 23 Jun 2022 17:36:57 -0700 Subject: [PATCH 33/39] add appHookData and hookData distinction + tests --- src/WorkerChannel.ts | 3 + src/eventHandlers/InvocationHandler.ts | 10 +- src/startApp.ts | 6 +- test/eventHandlers/InvocationHandler.test.ts | 221 +++++++++++++++++++ test/startApp.test.ts | 9 +- types-core/index.d.ts | 6 +- 6 files changed, 243 insertions(+), 12 deletions(-) diff --git a/src/WorkerChannel.ts b/src/WorkerChannel.ts index ecec0a20..78315746 100644 --- a/src/WorkerChannel.ts +++ b/src/WorkerChannel.ts @@ -16,7 +16,10 @@ export class WorkerChannel { functionLoader: IFunctionLoader; packageJson: PackageJson; hostVersion: string | undefined; + // this hook data will be passed to (and set by) all hooks in all scopes appHookData: HookData = {}; + // this hook data is limited to the app-level scope and persisted only for app-level hooks + appLevelOnlyHookData: HookData = {}; #preInvocationHooks: HookCallback[] = []; #postInvocationHooks: HookCallback[] = []; #appStartHooks: HookCallback[] = []; diff --git a/src/eventHandlers/InvocationHandler.ts b/src/eventHandlers/InvocationHandler.ts index f4a4d0e7..6feacfa9 100644 --- a/src/eventHandlers/InvocationHandler.ts +++ b/src/eventHandlers/InvocationHandler.ts @@ -94,12 +94,12 @@ export class InvocationHandler extends EventHandler<'invocationRequest', 'invoca // create a copy of the hook data in the worker context (set by app hooks) // the same hook data object is shared in the invocation hooks of the same invocation - const invocationHookData: HookData = { - ...channel.appHookData, - }; + const invocationHookData: HookData = {}; + const appHookData: HookData = channel.appHookData; const preInvocContext: PreInvocationContext = { hookData: invocationHookData, + appHookData, invocationContext: context, functionCallback: userFunction, inputs, @@ -122,7 +122,8 @@ export class InvocationHandler extends EventHandler<'invocationRequest', 'invoca } const postInvocContext: PostInvocationContext = { - hookData: invocationHookData, + hookData: preInvocContext.hookData, + appHookData: preInvocContext.appHookData, invocationContext: context, inputs, result: null, @@ -145,6 +146,7 @@ export class InvocationHandler extends EventHandler<'invocationRequest', 'invoca throw postInvocContext.error; } const result = postInvocContext.result; + channel.appHookData = postInvocContext.appHookData; // Allow HTTP response from context.res if HTTP response is not defined from the context.bindings object if (info.httpOutputName && context.res && context.bindings[info.httpOutputName] === undefined) { diff --git a/src/startApp.ts b/src/startApp.ts index 2ccc67c1..a7efa668 100644 --- a/src/startApp.ts +++ b/src/startApp.ts @@ -23,12 +23,14 @@ export async function startApp(functionAppDirectory: string, channel: WorkerChan await channel.updatePackageJson(functionAppDirectory); await loadEntryPointFile(functionAppDirectory, channel); const appStartContext: AppStartContext = { - hookData: channel.appHookData, + hookData: channel.appLevelOnlyHookData, + appHookData: channel.appHookData, functionAppDirectory, hostVersion: channel.hostVersion, }; await channel.executeHooks('appStart', appStartContext); - channel.appHookData = appStartContext.hookData; + channel.appHookData = appStartContext.appHookData; + channel.appLevelOnlyHookData = appStartContext.hookData; } async function loadEntryPointFile(functionAppDirectory: string, channel: WorkerChannel): Promise { diff --git a/test/eventHandlers/InvocationHandler.test.ts b/test/eventHandlers/InvocationHandler.test.ts index 309932c8..f0bf8c11 100644 --- a/test/eventHandlers/InvocationHandler.test.ts +++ b/test/eventHandlers/InvocationHandler.test.ts @@ -13,6 +13,7 @@ import { FunctionInfo } from '../../src/FunctionInfo'; import { FunctionLoader } from '../../src/FunctionLoader'; import { beforeEventHandlerSuite } from './beforeEventHandlerSuite'; import { TestEventStream } from './TestEventStream'; +import { Msg as WorkerInitMsg } from './WorkerInitHandler.test'; import LogCategory = rpc.RpcLog.RpcLogCategory; import LogLevel = rpc.RpcLog.Level; @@ -759,6 +760,226 @@ describe('InvocationHandler', () => { expect(hookData).to.equal('prepost'); }); + it('appHookData changes from appStart hooks are persisted in invocation hook contexts', async () => { + const functionAppDirectory = __dirname; + const expectedAppHookData = { + hello: 'world', + test: { + test2: 3, + }, + }; + const startFunc = sinon.spy((context: coreTypes.AppStartContext) => { + context.appHookData = expectedAppHookData; + hookData += 'appStart'; + }); + testDisposables.push(coreApi.registerHook('appStart', startFunc)); + + stream.addTestMessage(WorkerInitMsg.init(functionAppDirectory)); + + await stream.assertCalledWith( + WorkerInitMsg.receivedInitLog, + WorkerInitMsg.warning('Worker failed to load package.json: file does not exist'), + Msg.executingHooksLog(1, 'appStart'), + Msg.executedHooksLog('appStart'), + WorkerInitMsg.response + ); + expect(startFunc.callCount).to.be.equal(1); + + loader.getFunc.returns(async () => {}); + loader.getInfo.returns(new FunctionInfo(Binding.queue)); + + testDisposables.push( + coreApi.registerHook('preInvocation', (context: coreTypes.PreInvocationContext) => { + expect(context.appHookData).to.deep.equal(expectedAppHookData); + hookData += 'preInvoc'; + }) + ); + + testDisposables.push( + coreApi.registerHook('postInvocation', (context: coreTypes.PostInvocationContext) => { + expect(context.appHookData).to.deep.equal(expectedAppHookData); + hookData += 'postInvoc'; + }) + ); + + sendInvokeMessage([InputData.http]); + await stream.assertCalledWith( + Msg.receivedInvocLog(), + Msg.executingHooksLog(1, 'preInvocation'), + Msg.executedHooksLog('preInvocation'), + Msg.executingHooksLog(1, 'postInvocation'), + Msg.executedHooksLog('postInvocation'), + Msg.invocResponse([]) + ); + expect(hookData).to.equal('appStartpreInvocpostInvoc'); + }); + + it('hookData changes from appStart hooks are not persisted in invocation hook contexts', async () => { + const functionAppDirectory = __dirname; + const startFunc = sinon.spy((context: coreTypes.AppStartContext) => { + context.hookData = { + hello: 'world', + test: { + test2: 3, + }, + }; + hookData += 'appStart'; + }); + testDisposables.push(coreApi.registerHook('appStart', startFunc)); + + stream.addTestMessage(WorkerInitMsg.init(functionAppDirectory)); + + await stream.assertCalledWith( + WorkerInitMsg.receivedInitLog, + WorkerInitMsg.warning('Worker failed to load package.json: file does not exist'), + Msg.executingHooksLog(1, 'appStart'), + Msg.executedHooksLog('appStart'), + WorkerInitMsg.response + ); + expect(startFunc.callCount).to.be.equal(1); + + loader.getFunc.returns(async () => {}); + loader.getInfo.returns(new FunctionInfo(Binding.queue)); + + testDisposables.push( + coreApi.registerHook('preInvocation', (context: coreTypes.PreInvocationContext) => { + expect(context.hookData).to.be.empty; + expect(context.appHookData).to.be.empty; + hookData += 'preInvoc'; + }) + ); + + testDisposables.push( + coreApi.registerHook('postInvocation', (context: coreTypes.PostInvocationContext) => { + expect(context.hookData).to.be.empty; + expect(context.appHookData).to.be.empty; + hookData += 'postInvoc'; + }) + ); + + sendInvokeMessage([InputData.http]); + await stream.assertCalledWith( + Msg.receivedInvocLog(), + Msg.executingHooksLog(1, 'preInvocation'), + Msg.executedHooksLog('preInvocation'), + Msg.executingHooksLog(1, 'postInvocation'), + Msg.executedHooksLog('postInvocation'), + Msg.invocResponse([]) + ); + + expect(hookData).to.equal('appStartpreInvocpostInvoc'); + }); + + it('appHookData changes are persisted between invocation-level hooks', async () => { + const expectedAppHookData = { + hello: 'world', + test: { + test2: 3, + }, + }; + + loader.getFunc.returns(async () => {}); + loader.getInfo.returns(new FunctionInfo(Binding.queue)); + + testDisposables.push( + coreApi.registerHook('preInvocation', (context: coreTypes.PreInvocationContext) => { + context.appHookData = expectedAppHookData; + hookData += 'pre'; + }) + ); + + testDisposables.push( + coreApi.registerHook('postInvocation', (context: coreTypes.PostInvocationContext) => { + expect(context.appHookData).to.deep.equal(expectedAppHookData); + hookData += 'post'; + }) + ); + + sendInvokeMessage([InputData.http]); + await stream.assertCalledWith( + Msg.receivedInvocLog(), + Msg.executingHooksLog(1, 'preInvocation'), + Msg.executedHooksLog('preInvocation'), + Msg.executingHooksLog(1, 'postInvocation'), + Msg.executedHooksLog('postInvocation'), + Msg.invocResponse([]) + ); + + expect(hookData).to.equal('prepost'); + }); + + it('appHookData changes are persisted across different invocations while hookData changes are not', async () => { + const expectedAppHookData = { + hello: 'world', + test: { + test2: 3, + }, + }; + const expectedInvocationHookData = { + hello2: 'world2', + test2: { + test4: 5, + }, + }; + + loader.getFunc.returns(async () => {}); + loader.getInfo.returns(new FunctionInfo(Binding.queue)); + + const pre1 = coreApi.registerHook('preInvocation', (context: coreTypes.PreInvocationContext) => { + context.appHookData = expectedAppHookData; + context.hookData = expectedInvocationHookData; + hookData += 'pre1'; + }); + testDisposables.push(pre1); + + const post1 = coreApi.registerHook('postInvocation', (context: coreTypes.PostInvocationContext) => { + expect(context.appHookData).to.deep.equal(expectedAppHookData); + expect(context.hookData).to.deep.equal(expectedInvocationHookData); + hookData += 'post1'; + }); + testDisposables.push(post1); + + sendInvokeMessage([InputData.http]); + await stream.assertCalledWith( + Msg.receivedInvocLog(), + Msg.executingHooksLog(1, 'preInvocation'), + Msg.executedHooksLog('preInvocation'), + Msg.executingHooksLog(1, 'postInvocation'), + Msg.executedHooksLog('postInvocation'), + Msg.invocResponse([]) + ); + expect(hookData).to.equal('pre1post1'); + + pre1.dispose(); + post1.dispose(); + + const pre2 = coreApi.registerHook('preInvocation', (context: coreTypes.PreInvocationContext) => { + expect(context.appHookData).to.deep.equal(expectedAppHookData); + expect(context.hookData).to.be.empty; + hookData += 'pre2'; + }); + testDisposables.push(pre2); + + const post2 = coreApi.registerHook('postInvocation', (context: coreTypes.PostInvocationContext) => { + expect(context.appHookData).to.deep.equal(expectedAppHookData); + expect(context.hookData).to.be.empty; + hookData += 'post2'; + }); + testDisposables.push(post2); + + sendInvokeMessage([InputData.http]); + await stream.assertCalledWith( + Msg.receivedInvocLog(), + Msg.executingHooksLog(1, 'preInvocation'), + Msg.executedHooksLog('preInvocation'), + Msg.executingHooksLog(1, 'postInvocation'), + Msg.executedHooksLog('postInvocation'), + Msg.invocResponse([]) + ); + + expect(hookData).to.equal('pre1post1pre2post2'); + }); + it('dispose hooks', async () => { loader.getFunc.returns(async () => {}); loader.getInfo.returns(new FunctionInfo(Binding.queue)); diff --git a/test/startApp.test.ts b/test/startApp.test.ts index c5397d63..c7743d64 100644 --- a/test/startApp.test.ts +++ b/test/startApp.test.ts @@ -71,6 +71,7 @@ describe('startApp', () => { functionAppDirectory, hostVersion, hookData: {}, + appHookData: {}, }; const startFunc = sinon.spy(); @@ -97,6 +98,7 @@ describe('startApp', () => { functionAppDirectory, hostVersion, hookData: {}, + appHookData: {}, }; const startFunc = sinon.spy(); @@ -128,7 +130,7 @@ describe('startApp', () => { expect(startFunc.args[0][0]).to.deep.equal(expectedStartContext); }); - it('persists hookData changes from app start hooks in worker channel', async () => { + it('persists changes for app-level scope hookData', async () => { const functionAppDirectory = __dirname; const expectedHookData = { hello: 'world', @@ -152,9 +154,6 @@ describe('startApp', () => { ); expect(startFunc.callCount).to.be.equal(1); - expect(channel.appHookData).to.deep.equal(expectedHookData); + expect(channel.appLevelOnlyHookData).to.deep.equal(expectedHookData); }); - - it('passes app start hookData changes to invocation hooks', () => {}); - it('does not persist invocation hooks hookData changes', () => {}); }); diff --git a/types-core/index.d.ts b/types-core/index.d.ts index 8a4c6314..7749d797 100644 --- a/types-core/index.d.ts +++ b/types-core/index.d.ts @@ -33,9 +33,13 @@ declare module '@azure/functions-core' { */ export interface HookContext { /** - * The recommended place to share data between hooks + * The recommended place to share data between hooks in the same scope (app-level vs invocation-level) */ hookData: HookData; + /** + * The recommended place to share data across scopes for all hooks + */ + appHookData: HookData; } /** From 32da71170004775318ea955e642dc8644a580920 Mon Sep 17 00:00:00 2001 From: hossam-nasr Date: Thu, 23 Jun 2022 17:50:50 -0700 Subject: [PATCH 34/39] fix tests --- test/eventHandlers/InvocationHandler.test.ts | 15 ++++++++++----- test/startApp.test.ts | 4 +++- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/test/eventHandlers/InvocationHandler.test.ts b/test/eventHandlers/InvocationHandler.test.ts index f0bf8c11..e0d781ca 100644 --- a/test/eventHandlers/InvocationHandler.test.ts +++ b/test/eventHandlers/InvocationHandler.test.ts @@ -11,6 +11,8 @@ import * as sinon from 'sinon'; import { AzureFunctionsRpcMessages as rpc } from '../../azure-functions-language-worker-protobuf/src/rpc'; import { FunctionInfo } from '../../src/FunctionInfo'; import { FunctionLoader } from '../../src/FunctionLoader'; +import { WorkerChannel } from '../../src/WorkerChannel'; +import { Msg as AppStartMsg } from '../startApp.test'; import { beforeEventHandlerSuite } from './beforeEventHandlerSuite'; import { TestEventStream } from './TestEventStream'; import { Msg as WorkerInitMsg } from './WorkerInitHandler.test'; @@ -323,16 +325,19 @@ namespace InputData { describe('InvocationHandler', () => { let stream: TestEventStream; let loader: sinon.SinonStubbedInstance; + let channel: WorkerChannel; let coreApi: typeof coreTypes; let testDisposables: coreTypes.Disposable[] = []; before(async () => { - ({ stream, loader } = beforeEventHandlerSuite()); + ({ stream, loader, channel } = beforeEventHandlerSuite()); coreApi = await import('@azure/functions-core'); }); beforeEach(async () => { hookData = ''; + channel.appHookData = {}; + channel.appLevelOnlyHookData = {}; }); afterEach(async () => { @@ -779,8 +784,8 @@ describe('InvocationHandler', () => { await stream.assertCalledWith( WorkerInitMsg.receivedInitLog, WorkerInitMsg.warning('Worker failed to load package.json: file does not exist'), - Msg.executingHooksLog(1, 'appStart'), - Msg.executedHooksLog('appStart'), + AppStartMsg.executingHooksLog(1, 'appStart'), + AppStartMsg.executedHooksLog('appStart'), WorkerInitMsg.response ); expect(startFunc.callCount).to.be.equal(1); @@ -832,8 +837,8 @@ describe('InvocationHandler', () => { await stream.assertCalledWith( WorkerInitMsg.receivedInitLog, WorkerInitMsg.warning('Worker failed to load package.json: file does not exist'), - Msg.executingHooksLog(1, 'appStart'), - Msg.executedHooksLog('appStart'), + AppStartMsg.executingHooksLog(1, 'appStart'), + AppStartMsg.executedHooksLog('appStart'), WorkerInitMsg.response ); expect(startFunc.callCount).to.be.equal(1); diff --git a/test/startApp.test.ts b/test/startApp.test.ts index c7743d64..c9eb15b3 100644 --- a/test/startApp.test.ts +++ b/test/startApp.test.ts @@ -13,7 +13,7 @@ import { Msg as WorkerInitMsg } from './eventHandlers/WorkerInitHandler.test'; import LogCategory = rpc.RpcLog.RpcLogCategory; import LogLevel = rpc.RpcLog.Level; -namespace Msg { +export namespace Msg { export function executingHooksLog(count: number, hookName: string): rpc.IStreamingMessage { return { rpcLog: { @@ -62,6 +62,8 @@ describe('startApp', () => { coreApi.Disposable.from(...testDisposables).dispose(); testDisposables = []; process.chdir(originalCwd); + channel.appHookData = {}; + channel.appLevelOnlyHookData = {}; }); it('runs app start hooks in non-specialization scenario', async () => { From 21092d8ee5649ce46ea8e7181b3860496433edec Mon Sep 17 00:00:00 2001 From: hossam-nasr Date: Thu, 23 Jun 2022 18:02:03 -0700 Subject: [PATCH 35/39] remove outdated comment --- src/eventHandlers/InvocationHandler.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/eventHandlers/InvocationHandler.ts b/src/eventHandlers/InvocationHandler.ts index 6feacfa9..65795de6 100644 --- a/src/eventHandlers/InvocationHandler.ts +++ b/src/eventHandlers/InvocationHandler.ts @@ -92,8 +92,6 @@ export class InvocationHandler extends EventHandler<'invocationRequest', 'invoca let userFunction = channel.functionLoader.getFunc(functionId); - // create a copy of the hook data in the worker context (set by app hooks) - // the same hook data object is shared in the invocation hooks of the same invocation const invocationHookData: HookData = {}; const appHookData: HookData = channel.appHookData; From b8152ce00989e1da5d1f4018a2599bb2074425a0 Mon Sep 17 00:00:00 2001 From: hossam-nasr Date: Fri, 24 Jun 2022 14:23:06 -0700 Subject: [PATCH 36/39] non null hostVersion and remove log warning --- src/eventHandlers/WorkerInitHandler.ts | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/eventHandlers/WorkerInitHandler.ts b/src/eventHandlers/WorkerInitHandler.ts index 0e6213d5..a023ee2d 100644 --- a/src/eventHandlers/WorkerInitHandler.ts +++ b/src/eventHandlers/WorkerInitHandler.ts @@ -6,6 +6,7 @@ import * as path from 'path'; import { AzureFunctionsRpcMessages as rpc } from '../../azure-functions-language-worker-protobuf/src/rpc'; import { startApp } from '../startApp'; import { isError } from '../utils/ensureErrorType'; +import { nonNullProp } from '../utils/nonNull'; import { WorkerChannel } from '../WorkerChannel'; import { EventHandler } from './EventHandler'; import LogCategory = rpc.RpcLog.RpcLogCategory; @@ -32,18 +33,14 @@ export class WorkerInitHandler extends EventHandler<'workerInitRequest', 'worker logColdStartWarning(channel); + channel.hostVersion = nonNullProp(msg, 'hostVersion'); + if (msg.hostVersion) { channel.hostVersion = msg.hostVersion; } if (msg.functionAppDirectory) { await startApp(msg.functionAppDirectory, channel); - } else { - channel.log({ - message: 'functionAppDirectory was undefined in workerInitRequest message. Skipping startup logic.', - level: LogLevel.Warning, - logCategory: LogCategory.System, - }); } response.capabilities = { From 6b8c29830e797f478d6fab80cfff2e2262f9479c Mon Sep 17 00:00:00 2001 From: hossam-nasr Date: Mon, 27 Jun 2022 11:18:31 -0700 Subject: [PATCH 37/39] fix tests --- src/WorkerChannel.ts | 2 +- test/eventHandlers/WorkerInitHandler.test.ts | 2 +- test/startApp.test.ts | 30 +++++++++++--------- types-core/index.d.ts | 2 +- 4 files changed, 19 insertions(+), 17 deletions(-) diff --git a/src/WorkerChannel.ts b/src/WorkerChannel.ts index 78315746..399c89b0 100644 --- a/src/WorkerChannel.ts +++ b/src/WorkerChannel.ts @@ -15,7 +15,7 @@ export class WorkerChannel { eventStream: IEventStream; functionLoader: IFunctionLoader; packageJson: PackageJson; - hostVersion: string | undefined; + hostVersion = ''; // this hook data will be passed to (and set by) all hooks in all scopes appHookData: HookData = {}; // this hook data is limited to the app-level scope and persisted only for app-level hooks diff --git a/test/eventHandlers/WorkerInitHandler.test.ts b/test/eventHandlers/WorkerInitHandler.test.ts index 002a248a..a1b53350 100644 --- a/test/eventHandlers/WorkerInitHandler.test.ts +++ b/test/eventHandlers/WorkerInitHandler.test.ts @@ -16,7 +16,7 @@ import LogCategory = rpc.RpcLog.RpcLogCategory; import LogLevel = rpc.RpcLog.Level; export namespace Msg { - export function init(functionAppDirectory: string = __dirname, hostVersion?: string): rpc.IStreamingMessage { + export function init(functionAppDirectory: string = __dirname, hostVersion = '2.7.0'): rpc.IStreamingMessage { return { requestId: 'id', workerInitRequest: { diff --git a/test/startApp.test.ts b/test/startApp.test.ts index c9eb15b3..808347d3 100644 --- a/test/startApp.test.ts +++ b/test/startApp.test.ts @@ -132,30 +132,32 @@ describe('startApp', () => { expect(startFunc.args[0][0]).to.deep.equal(expectedStartContext); }); - it('persists changes for app-level scope hookData', async () => { + it('allows different appStart hooks to share data', async () => { const functionAppDirectory = __dirname; - const expectedHookData = { - hello: 'world', - test: { - test2: 3, - }, - }; - const startFunc = sinon.spy((context: coreTypes.AppStartContext) => { - context.hookData = expectedHookData; - }); - testDisposables.push(coreApi.registerHook('appStart', startFunc)); + let hookData = ''; + testDisposables.push( + coreApi.registerHook('appStart', (context) => { + context.hookData.hello = 'world'; + hookData += 'start1'; + }) + ); + testDisposables.push( + coreApi.registerHook('appStart', (context) => { + expect(context.hookData.hello).to.equal('world'); + hookData += 'start2'; + }) + ); stream.addTestMessage(WorkerInitMsg.init(functionAppDirectory)); await stream.assertCalledWith( WorkerInitMsg.receivedInitLog, WorkerInitMsg.warning('Worker failed to load package.json: file does not exist'), - Msg.executingHooksLog(1, 'appStart'), + Msg.executingHooksLog(2, 'appStart'), Msg.executedHooksLog('appStart'), WorkerInitMsg.response ); - expect(startFunc.callCount).to.be.equal(1); - expect(channel.appLevelOnlyHookData).to.deep.equal(expectedHookData); + expect(hookData).to.equal('start1start2'); }); }); diff --git a/types-core/index.d.ts b/types-core/index.d.ts index 7749d797..70750d61 100644 --- a/types-core/index.d.ts +++ b/types-core/index.d.ts @@ -101,7 +101,7 @@ declare module '@azure/functions-core' { /** * The version of the host running the function app */ - hostVersion?: string; + hostVersion: string; } /** From bbeb91692c51140dd2f16ca9b104c8fc505779b5 Mon Sep 17 00:00:00 2001 From: hossam-nasr Date: Mon, 27 Jun 2022 11:29:49 -0700 Subject: [PATCH 38/39] old hookData behavior --- src/eventHandlers/InvocationHandler.ts | 8 +++----- src/startApp.ts | 2 -- test/eventHandlers/InvocationHandler.test.ts | 12 ++++++------ 3 files changed, 9 insertions(+), 13 deletions(-) diff --git a/src/eventHandlers/InvocationHandler.ts b/src/eventHandlers/InvocationHandler.ts index 65795de6..9a55dd97 100644 --- a/src/eventHandlers/InvocationHandler.ts +++ b/src/eventHandlers/InvocationHandler.ts @@ -93,11 +93,10 @@ export class InvocationHandler extends EventHandler<'invocationRequest', 'invoca let userFunction = channel.functionLoader.getFunc(functionId); const invocationHookData: HookData = {}; - const appHookData: HookData = channel.appHookData; const preInvocContext: PreInvocationContext = { hookData: invocationHookData, - appHookData, + appHookData: channel.appHookData, invocationContext: context, functionCallback: userFunction, inputs, @@ -120,8 +119,8 @@ export class InvocationHandler extends EventHandler<'invocationRequest', 'invoca } const postInvocContext: PostInvocationContext = { - hookData: preInvocContext.hookData, - appHookData: preInvocContext.appHookData, + hookData: invocationHookData, + appHookData: channel.appHookData, invocationContext: context, inputs, result: null, @@ -144,7 +143,6 @@ export class InvocationHandler extends EventHandler<'invocationRequest', 'invoca throw postInvocContext.error; } const result = postInvocContext.result; - channel.appHookData = postInvocContext.appHookData; // Allow HTTP response from context.res if HTTP response is not defined from the context.bindings object if (info.httpOutputName && context.res && context.bindings[info.httpOutputName] === undefined) { diff --git a/src/startApp.ts b/src/startApp.ts index a7efa668..66797905 100644 --- a/src/startApp.ts +++ b/src/startApp.ts @@ -29,8 +29,6 @@ export async function startApp(functionAppDirectory: string, channel: WorkerChan hostVersion: channel.hostVersion, }; await channel.executeHooks('appStart', appStartContext); - channel.appHookData = appStartContext.appHookData; - channel.appLevelOnlyHookData = appStartContext.hookData; } async function loadEntryPointFile(functionAppDirectory: string, channel: WorkerChannel): Promise { diff --git a/test/eventHandlers/InvocationHandler.test.ts b/test/eventHandlers/InvocationHandler.test.ts index e0d781ca..70970025 100644 --- a/test/eventHandlers/InvocationHandler.test.ts +++ b/test/eventHandlers/InvocationHandler.test.ts @@ -774,7 +774,7 @@ describe('InvocationHandler', () => { }, }; const startFunc = sinon.spy((context: coreTypes.AppStartContext) => { - context.appHookData = expectedAppHookData; + Object.assign(context.appHookData, expectedAppHookData); hookData += 'appStart'; }); testDisposables.push(coreApi.registerHook('appStart', startFunc)); @@ -822,12 +822,12 @@ describe('InvocationHandler', () => { it('hookData changes from appStart hooks are not persisted in invocation hook contexts', async () => { const functionAppDirectory = __dirname; const startFunc = sinon.spy((context: coreTypes.AppStartContext) => { - context.hookData = { + Object.assign(context.hookData, { hello: 'world', test: { test2: 3, }, - }; + }); hookData += 'appStart'; }); testDisposables.push(coreApi.registerHook('appStart', startFunc)); @@ -888,7 +888,7 @@ describe('InvocationHandler', () => { testDisposables.push( coreApi.registerHook('preInvocation', (context: coreTypes.PreInvocationContext) => { - context.appHookData = expectedAppHookData; + Object.assign(context.appHookData, expectedAppHookData); hookData += 'pre'; }) ); @@ -931,8 +931,8 @@ describe('InvocationHandler', () => { loader.getInfo.returns(new FunctionInfo(Binding.queue)); const pre1 = coreApi.registerHook('preInvocation', (context: coreTypes.PreInvocationContext) => { - context.appHookData = expectedAppHookData; - context.hookData = expectedInvocationHookData; + Object.assign(context.appHookData, expectedAppHookData); + Object.assign(context.hookData, expectedInvocationHookData); hookData += 'pre1'; }); testDisposables.push(pre1); From 0564e9d36a1e22a8a40eb4069c996982b9703f07 Mon Sep 17 00:00:00 2001 From: hossam-nasr Date: Tue, 28 Jun 2022 16:04:26 -0700 Subject: [PATCH 39/39] address PR comments --- src/WorkerChannel.ts | 13 ++++++++++--- src/eventHandlers/WorkerInitHandler.ts | 4 ---- src/startApp.ts | 3 ++- .../FunctionEnvironmentReloadHandler.test.ts | 1 + 4 files changed, 13 insertions(+), 8 deletions(-) diff --git a/src/WorkerChannel.ts b/src/WorkerChannel.ts index 399c89b0..80cee7d2 100644 --- a/src/WorkerChannel.ts +++ b/src/WorkerChannel.ts @@ -15,10 +15,17 @@ export class WorkerChannel { eventStream: IEventStream; functionLoader: IFunctionLoader; packageJson: PackageJson; - hostVersion = ''; - // this hook data will be passed to (and set by) all hooks in all scopes + /** + * This will only be set after worker init request is received + */ + hostVersion?: string; + /** + * this hook data will be passed to (and set by) all hooks in all scopes + */ appHookData: HookData = {}; - // this hook data is limited to the app-level scope and persisted only for app-level hooks + /** + * this hook data is limited to the app-level scope and persisted only for app-level hooks + */ appLevelOnlyHookData: HookData = {}; #preInvocationHooks: HookCallback[] = []; #postInvocationHooks: HookCallback[] = []; diff --git a/src/eventHandlers/WorkerInitHandler.ts b/src/eventHandlers/WorkerInitHandler.ts index a023ee2d..7e83862f 100644 --- a/src/eventHandlers/WorkerInitHandler.ts +++ b/src/eventHandlers/WorkerInitHandler.ts @@ -35,10 +35,6 @@ export class WorkerInitHandler extends EventHandler<'workerInitRequest', 'worker channel.hostVersion = nonNullProp(msg, 'hostVersion'); - if (msg.hostVersion) { - channel.hostVersion = msg.hostVersion; - } - if (msg.functionAppDirectory) { await startApp(msg.functionAppDirectory, channel); } diff --git a/src/startApp.ts b/src/startApp.ts index 66797905..3e03c25c 100644 --- a/src/startApp.ts +++ b/src/startApp.ts @@ -6,6 +6,7 @@ import { pathExists } from 'fs-extra'; import { AzureFunctionsRpcMessages as rpc } from '../azure-functions-language-worker-protobuf/src/rpc'; import { loadScriptFile } from './loadScriptFile'; import { ensureErrorType } from './utils/ensureErrorType'; +import { nonNullProp } from './utils/nonNull'; import { WorkerChannel } from './WorkerChannel'; import path = require('path'); import LogLevel = rpc.RpcLog.Level; @@ -26,7 +27,7 @@ export async function startApp(functionAppDirectory: string, channel: WorkerChan hookData: channel.appLevelOnlyHookData, appHookData: channel.appHookData, functionAppDirectory, - hostVersion: channel.hostVersion, + hostVersion: nonNullProp(channel, 'hostVersion'), }; await channel.executeHooks('appStart', appStartContext); } diff --git a/test/eventHandlers/FunctionEnvironmentReloadHandler.test.ts b/test/eventHandlers/FunctionEnvironmentReloadHandler.test.ts index 8c86f791..4630521f 100644 --- a/test/eventHandlers/FunctionEnvironmentReloadHandler.test.ts +++ b/test/eventHandlers/FunctionEnvironmentReloadHandler.test.ts @@ -77,6 +77,7 @@ describe('FunctionEnvironmentReloadHandler', () => { originalCwd = process.cwd(); originalEnv = process.env; ({ stream, channel } = beforeEventHandlerSuite()); + channel.hostVersion = '2.7.0'; }); after(() => {