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

Fix out-of-sync binding names causing null ref error #216

Merged
merged 2 commits into from
Feb 5, 2024
Merged

Conversation

ejizba
Copy link
Contributor

@ejizba ejizba commented Feb 2, 2024

Fixes #210

The logs from the user indicate a mismatch in the binding name between the host and worker ("httpTrigger17" vs "httpTrigger19"). I suspect our logic in "addBindingName" isn't reliable enough at returning the same binding name. When I first adjusted the logic in "addBindingName" (see the original issue) I thought the problem only repro-ed with multiple workers in parallel, but now I think it can also happen with multiple workers in serial (aka a worker restarts, but the host stays the same). Fwiw, I checked telemetry and only a handful of apps are hitting this issue, but I definitely don't want anyone to hit this at all.

Anyways, my new solution is to create a hash of the binding configuration, which should reliably create the same binding name each time and also be unique within the function (we need name uniqueness at the function level, but not the app level). The only way we could get conflicting names is if they register bindings with the exact same config within a single function, but I have no idea why someone would do that. Regardless, I added an error up front so that it will fail-fast if they're in that situation and be easy for them to fix.

Lastly, I split this PR into two commits for easier review:

  • The first moves code to a new file with no functional changes: 2e24f14
  • The second actually fixes the issue: 2f08eb6

@ejizba ejizba requested a review from castrodd February 2, 2024 23:56
@ejizba ejizba merged commit 3822bc3 into v4.x Feb 5, 2024
11 checks passed
@ejizba ejizba deleted the ej/bindingName branch February 5, 2024 23:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cannot read properties of undefined (reading 'type')
2 participants