Azure / azure-functions-nodejs-worker

The Node.js worker for the Azure Functions runtime - https://functions.azure.com
MIT License
106 stars 44 forks source link

Improve Node.js Functions programming model 🚀 #480

Closed ejizba closed 1 year ago

ejizba commented 2 years ago

Goals

  1. Be idiomatic for Node.js developers. Users should be able to look at the code and immediately know what's going on
  2. Let users define their own file structure (related to https://github.com/Azure/azure-functions-host/issues/5373)
  3. Let users define function metadata in js/ts files instead of json
  4. Take advantage of types as much as possible, including for JavaScript users (i.e. VS Code can provide Intellisense for JavaScript based on the TypeScript types as long as it can find them)
  5. Model after an existing popular/modern framework (Fastify, Express, Firebase, Remix, etc.), but ideally be flexible enough to allow for other frameworks to be used

Plan of action

Moved to here: https://github.com/Azure/azure-functions-nodejs-worker/wiki/Roadmap#new-programming-model

Prototypes

See this repo for the example prototype: https://github.com/ejizba/func-nodejs-prototype

aaronpowell commented 2 years ago

One of the hardest things to handle is bindings - there's nothing like .NET attributes that you can leverage, so there's really two options IMO - object literal or a fluent API.

Here's some rough prototype ideas:

import app from "@azure/functions";

// object literal
app.register(async (context: FunctionContext) => {
  context.res = {
    body: { hello: "world" }
  };
}, {(
  bindings: [{
    type: "http",
    direction: "in",
    name: "req",
    methods: ["get", "post"]
  }, {
    type: "http",
    direction: "out",
    name: "res"
  }],
  route: "/echo"
});

// fluent API
app.register(async (context: FunctionContext) => {
  context.res = {
    body: { hello: "world" }
  }
}).binding({
    type: "http",
    direction: "in",
    name: "req",
    methods: ["get", "post"]
}).binding({
    type: "http",
    direction: "out",
    name: "res"
}).route("echo");

Both of those are a "low level API" and I don't see why you couldn't have something more like app.registerHttp(handler: AzureFunction, route: string, methods?: ("get" | "post" | "patch" | ...)[], additionalBindings?: FunctionBinding[]) as a method which does the sensible defaults but allows more stuff to be added.

I've also only done HTTP here, but there's nothing HTTP-centric around this model.

aaronpowell commented 2 years ago

I'd be looking at libraries like Fastify/Express/etc. as inspirations for how to create the API surface area, but not try and mimic them completely as they are centred so much around HTTP only, when you want to make a programming model that fits nicely for other bindings too.

ryancole commented 2 years ago

I feel honored to be in the bullet list of items that triggers this issue. JK. JK. :P

sinedied commented 2 years ago

With the prominence of TypeScript, I would suggest thinking of how the new API model could leverage TS to make it a first class citizen. For example, I would push forward TS decorators as a way to define bindings and other functions settings. I know decorators' TC39 proposal is currently only stage 2, and TS decorators are already diverging a bit from that proposal. But as syntactic sugar, they're unmatched and AFAIK all most popular TS-first frameworks tools use decorators: Angular, NestJS, TypeORM, TypeGraphQL, Loopback, Ts.ED...

This is not necessarily incompatible with @ejizba's prototype approach, but could come in addition to the regular syntax:

import { Http, Get, AuthLevel, FunctionContext } from '@azure/functions';

@Http('/hello')
@Get()
@AuthLevel('anonymous')
export async function ({ req, res }: FunctionContext) {
  res.body = 'Hello World!';
}

One key point is see would be to provide sensible defaults to reduce boilerplate (as @aaronpowell said), like in my example above the @Http() decorator would set in/out bindings associated to req and res, which is probably the most common use case.

Such an approach regarding default would also work with plain JS using functions:

import { app, FunctionContext } from '@azure/functions';

app.http('/hello', { methods: ['get'], authLevel: 'anonymous' },
  async ({ req, res }: FunctionContext) => {
    res.body = 'Hello World!';
  });

Or with a fluent api:

import { app, FunctionContext } from '@azure/functions';

app.http('/hello')
  .methods(['get'])
  .authLevel('anonymous')
  .register({ req, res }: FunctionContext) => {
    res.body = 'Hello World!';
  });

Of course, the full verbose would still be accessible, but reducing the amount of boilerplate for common use cases is something to not be overlooked.

aaronpowell commented 2 years ago

I would push forward TS decorators as a way to define bindings and other functions settings

I'm going to disagree with this. From the conversations that I've had with the TypeScript team, I felt that they were not really encouraging new projects to use the decorators that are in TypeScript because, as you pointed out, they aren't in line with the spec and if/when decorators do make it into the spec, they won't be able to directly port over the current implementation, so they'll have to have two different implementations within the language itself.

sinedied commented 2 years ago

The first goal is to be idiomatic for Node.js dev, and the fact is that decorators are hugely popular in all TS-first Node.js libs. This is what I think is the most important point here.

Now, I don't say that the API should be built exclusively around decorators, but we can use them as syntactic sugar around the base API. When/if the new decorators spec will be defined, the whole TS ecosystem would have to think about a migration path, and the TS compiler won't drop support for the old spec the next day.

ejizba commented 2 years ago

I understand there may be technical reasons decorators would be difficult for us (as @aaronpowell said), but I do plan to explore that option and have at least one prototype with decorators. I make no promises, but I think it's worth considering

anfibiacreativa commented 2 years ago

The first goal is to be idiomatic for Node.js dev, and the fact is that decorators are hugely popular in all TS-first Node.js libs. This is what I think is the most important point here.

Now, I don't say that the API should be built exclusively around decorators, but we can use them as syntactic sugar around the base API. When/if the new decorators spec will be defined, the whole TS ecosystem would have to think about a migration path, and the TS compiler won't drop support for the old spec the next day.

I support exploring the decorators option. Makes a lot of sense to me.

aaronpowell commented 2 years ago

With decorators progressing to Stage 3 last week, it's probably more viable to consider, but we'd want to leverage the design of decorators that's spec'ed out rather than the one in TypeScript today (and that might make things harder).

sedot42 commented 2 years ago

Wouldn't TS decorators be just a nice potential feature of the new programming model? Imo it sounds like a good compromise to keep this feature in mind and focus on getting the basics of a good programming model right.

I like the brevity of the high-level API form suggested here by @sinedied, leveraging the low-level API proposal of @aaronpowell.

import { app, FunctionContext } from '@azure/functions';

app.http('/hello', { methods: ['get'], authLevel: 'anonymous' },
  async ({ req, res }: FunctionContext) => {
    res.body = 'Hello World!';
  });
aaronpowell commented 2 years ago

I like the idea of decorators in principle but until there's support for the proposal that's going forward not the legacy one in TypeScript and Babel/swc/etc. I see it as being a really hard thing to support as a first-class citizen.

ChuckJonas commented 2 years ago

@aaronpowell IMO the object literal approach is where you should start.

Once you have that, community members interested in decorators could roll their own package. Allowing functions to be registered via code really opens the door to a lot of potential solutions.

IMO, there was only support for fluent API, it makes it a bit harder to extend.

ejizba commented 2 years ago

Hi folks! We've actually narrowed it down to one option, and it's a working prototype and everything. More details here if you're interested in giving some feedback: https://aka.ms/AzFuncNodeV4

@ChuckJonas in terms of decorators, it's definitely been on the back burner for now. We want to prototype it still, but we've been focused on a non-decorators approach as described in the link above.

ChuckJonas commented 2 years ago

@ejizba just gave it a review. I like where this is going!

Honestly the thing I'm most excited about is just being able to specify whatever folder structure fits the project best and not have every function as a folder in the root.

Our project is using OpenAPI specs to define our API contracts along side each function. We use openapi-typescript to get accurate request typings and validate all request using against the spec using ajv.

Overall, I think it's a pretty nice approach, but the limitations around function registration really make it harder to maintain than it would otherwise be.

Some feedback below:

Define function in code

  1. Is the first parameter the route? Or just the function name? I'm hoping it's the former as this would make it similar to other popular JS frameworks.

  2. Do you have an example Service bus triggered function?

Simplified context, inputs, and outputs

The context object has been simplified to reduce duplication and make it easier to write unit tests

That will be very welcome. Been burned by missing the context duplication in our test setup a couple times now.

Are there any plans to improve Service Bus IN/OUT formats? Guessing that might be outside the scope of this project, but I have two major problems with how service bus bindings work in AZ Function:

  1. in: input provides only the message.body. You have to go to the binding read the message properties / applicationProperties.

  2. out As far as I can tell, there are no ways to set anything other than the message body. This makes the out bindings a no-go for all but the simplest use cases (no sessionId, applicationProperties, etc)

Create a test context

Will be nice to have this out of the box... We basically rolled out our version of something very similar in our current project.

Question: Rules around function registration

Just curious in general how the function registration works...

PS: IMO, decorators don't bring much other than just making the code read closer what whats done in C#. It feels like the ts community has shifted away from OOO in favor of more functional code (decorators are only supported on classes). Current approach seems more flexible and composable.

ejizba commented 2 years ago

@ChuckJonas thanks for the feedback! I think I've covered everything:

Is the first parameter the route? Or just the function name? I'm hoping it's the former as this would make it similar to other popular JS frameworks.

It's the function name. Created this issue to continue the discussion on this: https://github.com/Azure/azure-functions-nodejs-library/issues/17

Do you have an example Service bus triggered function?

Yeah here are my example functions with a few service bus ones: https://github.com/ejizba/func-nodejs-prototype/tree/main/src/functions. Make sure to read the readme of that repo if you want to know how to try it out

Are there any plans to improve Service Bus IN/OUT formats?

No there are not. Agreed it's probably out of scope, as I think your problems are common across all languages, right? Regardless, feel free to file a separate issue if you want us to follow up

Can functions be conditional registered?

Yes, for example I added some conditions to only register a function if the respective environment variable is set in my example prototype. (see here)

Is there anything that stops a function from registering another function?

That is not possible. If you try it, you will get an error like "A function can only be registered during app startup."

IMO, decorators...

I've created a separate issue to start tracking any decorators work/discussion: https://github.com/Azure/azure-functions-nodejs-library/issues/18

ChuckJonas commented 1 year ago

@ejizba Ran into an issue today with distributed tracing that I was wondering if you guys would be solving via this initiative.

The problem is outlined here.

IDK if maybe this could be solved with the new hooks. I am using appinsights wrapped context, but you end up needing to rewrite all the context.log functionality because its operation id doesn't get updated.


export default async function contextPropagatingHttpTrigger(context, req) {
  // overwrite with the proper traceparent from the incoming SB message. 
  const sbTraceParent = context.bindingData.applicationProperties['diagnostic-Id'];
  if (sbTraceParent) {
    context.traceContext.traceparent = sbTraceParent;
  }

  const correlationContext = appInsights.startOperation(context, req) as CorrelationContext;

  // Wrap the Function runtime with correlationContext
  return appInsights.wrapWithCorrelationContext(async () => {
    const startTime = Date.now(); // Start trackRequest timer

    try {
      // operation_Id matches SB 'diagnostic-Id'
      appInsights.defaultClient.trackTrace({
        message: 'Correct Trace Context',
      });

      // operation_Id is the original function traceparent
      context.log('Incorrect Trace Context');

      return await trigger(context, req);

    } catch (e) {
      context.log.error(e);
      throw e;
    } finally {
      // Track Request on completion
      appInsights.defaultClient.flush();
    }
  }, correlationContext)();
}
ejizba commented 1 year ago

We're in public preview! 🎉 Try it out now and let us know what you think - read the blog post here: https://techcommunity.microsoft.com/t5/apps-on-azure-blog/azure-functions-version-4-of-the-node-js-programming-model-is-in/ba-p/3773541

I'm going to close this issue and we will track any GA work with its own individual issue