Proposal: Extending hooks

[aaronpowell]

While I've been experimenting with #480 I've been thinking about how we can create a "middleware" style feature using the hooks that were recently added to the worker. The ideas here span both the worker and the library repos.

Evolving the types

Looking at the types as defined here

azure-functions-nodejs-worker/types-core/index.d.ts

Lines 52 to 62 in f6d3625

	function registerHook(hookName: 'preInvocation', callback: PreInvocationCallback): Disposable;
	function registerHook(hookName: 'postInvocation', callback: PostInvocationCallback): Disposable;
	function registerHook(hookName: 'appStart', callback: AppStartCallback): Disposable;
	function registerHook(hookName: 'appTerminate', callback: AppTerminateCallback): Disposable;
	function registerHook(hookName: string, callback: HookCallback): Disposable;
	type HookCallback = (context: HookContext) => void | Promise<void>;
	type PreInvocationCallback = (context: PreInvocationContext) => void | Promise<void>;
	type PostInvocationCallback = (context: PostInvocationContext) => void | Promise<void>;
	type AppStartCallback = (context: AppStartContext) => void | Promise<void>;
	type AppTerminateCallback = (context: AppTerminateContext) => void | Promise<void>;

There isn't much in common with them, the type of hook to be registered are strings in the overload and the types for each callback don't have a common base type. As a result, trying to create a generic middleware function in the library project can be difficult without a lot of overloads there to ensure that the type system is happy.

Using an enum or union for the name of the hook would make it simpler to have other places within the code be aware of valid hook names.

For the callbacks, it'd be helpful if they inherited a base callback type as it can make for simpler type signatures elsewhere where you might want to create an abstraction layer over the hooks (again, something that could be exposed in the new programming model).

Adding more richness to HookContext

Is it possible to add some proper types to the PreInvocationContext and PostInvocationContext, particularly for the inputs? That would make it easier to understand what the inputs are (obviously you'd have to do your own explicit type checking as your inputs are inferred at runtime)? Maybe that could be a generic argument so that the usage of the hook could be explicit about the input types it's expecting?

I'm thinking in the new programming model we could support "hook filters", where you specify that you only want the hook to run on a certain trigger type, or when an input contains some value, etc.

For this we might also need more information on the HookContext, particularly the pre/post versions.

Do we know the "id" of the Function being executed? Can that be made available on the context object?

What about the trigger? Can the trigger be an explicit field so that you can write filters against "only HTTP triggered Functions"?

Pass data from hooks to Functions

One use-case for hooks is to be able to add additional stuff to a Function, say a CosmosClient so that the Function can do operations against CosmosDB that aren't possible via a binding.

Initially, it might seem like that's something you could use the hookData property for, but that's only persisted across the hooks, and not provided to the Function.

It is possible to use the InvocationContext as a way to pass stuff, but it's typed as unknown (probably so it can support either programming model) so you have to cast it to something else to work with. Here's how I did it with the new programming model:

registerHook("preInvocation", async (context) => {
  const client = new CosmosClient(process.env.CosmosConnectionString);
  const { database } = await client.databases.createIfNotExists({
    id: "TodoList",
  });
  const { container } = await database.containers.createIfNotExists({
    id: "Items",
    partitionKey: { paths: ["/id"] },
  });

  const ctx = context.invocationContext as any;
  ctx.extraInputs.set("cosmosContainer", container);
});

But if instead we had an explicity method, like setFunctionData on the hook context object, then we could write hooks that are simplified across the different programming models.

Ability to cancel Function runs

When thinking about hooks as middleware, there are reasons that you might want to cancel an execution of a Function if a pre-condition isn't met. An example of this would be for validation of an incoming HTTP trigger payload. While this could be done at a Function level, being able to use hooks as a way to make it generic really increases their usefulness.

This can be achieved by replacing the functionCallback on the hook context with a new function, but it's a bit hacky (and if anything changed in how the worker uses that property it may break).

Having an abort method on the InvocationContext would be a way to bail out.

[aaronpowell]

Added two more ideas after doing some more experiments with a more complex application

[ejizba]

we could write hooks that are simplified across the different programming models.

Can you expand on this? Our plan was to discourage people from using hooks from the worker directly. We want to keep the worker as a backwards-compatible, barebones api and I think we'd put most of the features you're asking for in the library package. Azure/azure-functions-nodejs-library#7

[aaronpowell]

The current folder-based approach for Functions isn't going to be going away, so there'll be a need to support writing something in that manner as well as something that leverages the library package.

Let's say you're building a library to work with MongoDB. You'd add a hook that establishes the connection before an invocation, but since it's a generic library you wouldn't want to tie it to any specific approach to writing Functions. So you need a generic way to provide the Mongo connection, but there isn't a way to do that at present.

[ejizba]

The folder-based approach can still be used in conjunction with the library package (usage here). v4 is the priority for adding hooks, but I think we were still planning on adding them to v3 of the package.

To me, the open question is how much we want to support the scenario you mention where a generic library doesn't want to tie themselves to a specific major version of the library package. I'm open to ideas, but hesitant to add a bunch of stuff in the worker. We really want to keep this api barebones and punt all the fancy/nice logic to the library packages.

[aaronpowell]

Would there be much need to change the hooks in the worker though? If we were to leverage hookData then it's already there, it's just passing that to the invocation channel rather, which is admittedly a change but it's then downstream of the channel to do something with that argument.

Otherwise if we made the types more obvious then it'd be apparent that the invocationContext could be used for it (which is the approach I've taken).

[ejizba]

Would there be much need to change the hooks in the worker though?

I'm confused, because I thought all the above requests were for the worker. Perhaps this would be easier if we split this into separate issues. Here is how I would split it up and the repo I think each should be assigned to:

• Evolving the types - worker
• Adding more richness to HookContext (everything except hook filters) - library
• Hook filters - library (I think this one deserves its own discussion)
• Pass data from hooks to Functions - library
• Ability to cancel Function runs - worker

I'll give you first crack at splitting the issues up, but I'm happy to do it myself if that's easier for you.

[aaronpowell]

Sure thing, I'll split them out as children of this one. I mostly started writing this as a collection of ideas for discussion and it's sort of expanded from that and now probably warrants being broken down for more concise discussions.

[aaronpowell]

Regarding the HookContext and filtering - is that something that would live in library or in the worker?

Is the "plumbing" needed for that is only in the library, won't we be restricting the feature to only work in that implementation and any other implementations that get created would have to replicate it? After all, the things you'd likely filter on (name of function, trigger type, additional input bindings, etc.) are common across all implementations of a library.

[ejizba]

Filtering is already possible which is why I don't think we need to do anything in the worker. Things like the name of the function, trigger type, etc. are already available on the invocationContext and users can filter themselves with a simple if case. I acknowledge we could make filtering easier, but the design for that will be pretty subjective and thus I feel like that discussion belongs in the library.

At the end of the day, we want people directly referencing the library, so no they shouldn't have to replicate anything. We don't want people using the core api directly except in very rare cases.

[aaronpowell]

Oh, I didn't realise that all that was available off the invocationContext - possibly with #665 can help address the discoverability of that.