Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Name uniqueness isn't enforced with the new programming model #62

Closed
aaronpowell opened this issue Mar 7, 2023 · 2 comments · Fixed by Azure/azure-functions-nodejs-worker#692
Labels
breaking enhancement GA blocker v4 🚀 We no longer use this label since v4 is now the default model.
Milestone

Comments

@aaronpowell
Copy link
Contributor

Since the developer provides the name of a Function in the new programming model, rather than it coming from the file system, it's possible to register multiple Functions with the same name:

import { app } from "@azure/functions";

app.get("todo", () => {});
app.post("todo", () => {});

In this example you're expecting that todo is the route (which it would be set as) and we have registered different verbs on the same route. But since the first argument is both name and route, only the last one is registered.

I see there being two options:

  1. We take the user-provided name and add some additional context information, e.g.: http-todo-get, where it's <trigger>-<name>-<additional info if required>.
  2. We either raise an error if a duplicate name is detected or warn in logs that only the last registered with that name will be registered.

Option 1 does a better job at enforcing unique names but with the drawback that it won't be as obvious in the logs until people realise the name they provided is only part of it. Option 2 avoids "magic naming" but with the drawback of a runtime warning or error, which might not be the ideal DX.

@ejizba
Copy link
Contributor

ejizba commented Mar 7, 2023

This is an interesting case. I'm not super excited about 1 - I think that's probably a bit too magic-y for me. For 2, I prefer we throw an error - this would make the app fail-fast (hopefully forcing the user to address the problem), but users could still catch & ignore the error if they really wanted. The other option is to directly log a warning/error instead of throwing it, but IMO this will just clutter the logs without forcing the user to do anything about it.

@ejizba ejizba added enhancement v4 🚀 We no longer use this label since v4 is now the default model. labels Mar 7, 2023
@aaronpowell
Copy link
Contributor Author

Yeah, I agree that throwing an error is the best approach as it avoids any ambiguity on why a function didn't get registered.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking enhancement GA blocker v4 🚀 We no longer use this label since v4 is now the default model.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants