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

src/addBindingName.ts

@@ -1,13 +1,15 @@
 // Copyright (c) .NET Foundation. All rights reserved.
 // Licensed under the MIT License.
 
-const bindingCounts: Record<string, number> = {};
+import { getStringHash } from './utils/getRandomHexString';
+
 /**
  * If the host spawns multiple workers, it expects the metadata (including binding name) to be the same across workers.
- * That means we need to generate binding names in a deterministic fashion, so we'll do that using a count
- * There's a tiny risk users register bindings in a non-deterministic order (i.e. async race conditions), but it's okay considering the following:
- * 1. We will track the count individually for each binding type. This makes the names more readable and reduces the chances a race condition will matter
- * 2. Users can manually specify the name themselves (aka if they're doing weird async stuff) and we will respect that
+ * That means we need to generate binding names in a deterministic fashion, so we'll do that using a string hash of the binding data
+ * A few considerations:
+ * 1. We will include the binding type in the name to make it more readable
+ * 2. Users can manually specify the name themselves and we will respect that
+ * 3. The only time the hash should cause a conflict is if a single function has duplicate bindings. Not sure why someone would do that, but we will throw an error at function registration time
  * More info here: https://github.com/Azure/azure-functions-nodejs-worker/issues/638
  */
 export function addBindingName<T extends { type: string; name?: string }>(
@@ -19,10 +21,7 @@ export function addBindingName<T extends { type: string; name?: string }>(
         if (!bindingType.toLowerCase().endsWith(suffix.toLowerCase())) {
             bindingType += suffix;
         }
-        let count = bindingCounts[bindingType] || 0;
-        count += 1;
-        bindingCounts[bindingType] = count;
-        binding.name = bindingType + count.toString();
+        binding.name = bindingType + getStringHash(JSON.stringify(binding));
     }
     return <T & { name: string }>binding;
 }

src/app.ts

@@ -5,8 +5,6 @@ import {
     CosmosDBFunctionOptions,
     EventGridFunctionOptions,
     EventHubFunctionOptions,
-    ExponentialBackoffRetryOptions,
-    FixedDelayRetryOptions,
     FunctionOptions,
     FunctionTrigger,
     GenericFunctionOptions,
@@ -21,14 +19,11 @@ import {
     TimerFunctionOptions,
     WarmupFunctionOptions,
 } from '@azure/functions';
-import * as coreTypes from '@azure/functions-core';
 import { FunctionCallback } from '@azure/functions-core';
-import { returnBindingKey } from './constants';
-import { toRpcDuration } from './converters/toRpcDuration';
+import { toCoreFunctionMetadata } from './converters/toCoreFunctionMetadata';
 import * as output from './output';
 import { ProgrammingModel } from './ProgrammingModel';
 import * as trigger from './trigger';
-import { isTrigger } from './utils/isTrigger';
 import { tryGetCoreApiLazy } from './utils/tryGetCoreApiLazy';
 
 export * as hook from './hooks/registerHook';
@@ -138,63 +133,12 @@ export function generic(name: string, options: GenericFunctionOptions): void {
         setProgrammingModel();
     }
 
-    const bindings: Record<string, coreTypes.RpcBindingInfo> = {};
-
-    const trigger = options.trigger;
-    bindings[trigger.name] = {
-        ...trigger,
-        direction: 'in',
-        type: isTrigger(trigger.type) ? trigger.type : trigger.type + 'Trigger',
-    };
-
-    if (options.extraInputs) {
-        for (const input of options.extraInputs) {
-            bindings[input.name] = {
-                ...input,
-                direction: 'in',
-            };
-        }
-    }
-
-    if (options.return) {
-        bindings[returnBindingKey] = {
-            ...options.return,
-            direction: 'out',
-        };
-    }
-
-    if (options.extraOutputs) {
-        for (const output of options.extraOutputs) {
-            bindings[output.name] = {
-                ...output,
-                direction: 'out',
-            };
-        }
-    }
-
-    let retryOptions: coreTypes.RpcRetryOptions | undefined;
-    if (options.retry) {
-        retryOptions = {
-            ...options.retry,
-            retryStrategy: options.retry.strategy,
-            delayInterval: toRpcDuration((<FixedDelayRetryOptions>options.retry).delayInterval, 'retry.delayInterval'),
-            maximumInterval: toRpcDuration(
-                (<ExponentialBackoffRetryOptions>options.retry).maximumInterval,
-                'retry.maximumInterval'
-            ),
-            minimumInterval: toRpcDuration(
-                (<ExponentialBackoffRetryOptions>options.retry).minimumInterval,
-                'retry.minimumInterval'
-            ),
-        };
-    }
-
     const coreApi = tryGetCoreApiLazy();
     if (!coreApi) {
         console.warn(
             `WARNING: Skipping call to register function "${name}" because the "@azure/functions" package is in test mode.`
         );
     } else {
-        coreApi.registerFunction({ name, bindings, retryOptions }, <FunctionCallback>options.handler);
+        coreApi.registerFunction(toCoreFunctionMetadata(name, options), <FunctionCallback>options.handler);
     }
 }

src/converters/toCoreFunctionMetadata.ts

@@ -0,0 +1,76 @@
+// Copyright (c) .NET Foundation. All rights reserved.
+// Licensed under the MIT License.
+
+import { ExponentialBackoffRetryOptions, FixedDelayRetryOptions, GenericFunctionOptions } from '@azure/functions';
+import * as coreTypes from '@azure/functions-core';
+import { returnBindingKey } from '../constants';
+import { AzFuncSystemError } from '../errors';
+import { isTrigger } from '../utils/isTrigger';
+import { toRpcDuration } from './toRpcDuration';
+
+export function toCoreFunctionMetadata(name: string, options: GenericFunctionOptions): coreTypes.FunctionMetadata {
+    const bindings: Record<string, coreTypes.RpcBindingInfo> = {};
+    const bindingNames: string[] = [];
+
+    const trigger = options.trigger;
+    bindings[trigger.name] = {
+        ...trigger,
+        direction: 'in',
+        type: isTrigger(trigger.type) ? trigger.type : trigger.type + 'Trigger',
+    };
+    bindingNames.push(trigger.name);
+
+    if (options.extraInputs) {
+        for (const input of options.extraInputs) {
+            bindings[input.name] = {
+                ...input,
+                direction: 'in',
+            };
+            bindingNames.push(input.name);
+        }
+    }
+
+    if (options.return) {
+        bindings[returnBindingKey] = {
+            ...options.return,
+            direction: 'out',
+        };
+        bindingNames.push(returnBindingKey);
+    }
+
+    if (options.extraOutputs) {
+        for (const output of options.extraOutputs) {
+            bindings[output.name] = {
+                ...output,
+                direction: 'out',
+            };
+            bindingNames.push(output.name);
+        }
+    }
+
+    const dupeBindings = bindingNames.filter((v, i) => bindingNames.indexOf(v) !== i);
+    if (dupeBindings.length > 0) {
+        throw new AzFuncSystemError(
+            `Duplicate bindings found for function "${name}". Remove a duplicate binding or manually specify the "name" property to make it unique.`
+        );
+    }
+
+    let retryOptions: coreTypes.RpcRetryOptions | undefined;
+    if (options.retry) {
+        retryOptions = {
+            ...options.retry,
+            retryStrategy: options.retry.strategy,
+            delayInterval: toRpcDuration((<FixedDelayRetryOptions>options.retry).delayInterval, 'retry.delayInterval'),
+            maximumInterval: toRpcDuration(
+                (<ExponentialBackoffRetryOptions>options.retry).maximumInterval,
+                'retry.maximumInterval'
+            ),
+            minimumInterval: toRpcDuration(
+                (<ExponentialBackoffRetryOptions>options.retry).minimumInterval,
+                'retry.minimumInterval'
+            ),
+        };
+    }
+
+    return { name, bindings, retryOptions };
+}

src/utils/getRandomHexString.ts

@@ -7,3 +7,7 @@ export function getRandomHexString(length = 10): string {
     const buffer: Buffer = crypto.randomBytes(Math.ceil(length / 2));
     return buffer.toString('hex').slice(0, length);
 }
+
+export function getStringHash(data: string, length = 10): string {
+    return crypto.createHash('sha256').update(data).digest('hex').slice(0, length);
+}

test/converters/toCoreFunctionMetadata.test.ts

@@ -0,0 +1,157 @@
+// Copyright (c) .NET Foundation. All rights reserved.
+// Licensed under the MIT License.
+
+import 'mocha';
+import { expect } from 'chai';
+import { output, trigger } from '../../src';
+import { toCoreFunctionMetadata } from '../../src/converters/toCoreFunctionMetadata';
+
+describe('toCoreFunctionMetadata', () => {
+    const handler = () => {};
+    const expectedHttpTrigger = {
+        authLevel: 'anonymous',
+        methods: ['GET', 'POST'],
+        type: 'httpTrigger',
+        name: 'httpTrigger433d175fc9',
+        direction: 'in',
+    };
+    const expectedHttpOutput = { type: 'http', name: 'httpOutput9a706511b1', direction: 'out' };
+    const expectedQueueOutput1 = {
+        queueName: 'e2e-test-queue-trigger',
+        connection: 'e2eTest_storage',
+        type: 'queue',
+        name: 'queueOutput8b95495f3d',
+        direction: 'out',
+    };
+
+    it('http trigger', () => {
+        const result = toCoreFunctionMetadata('funcName', {
+            handler,
+            trigger: trigger.http({}),
+            return: output.http({}),
+        });
+        expect(result).to.deep.equal({
+            name: 'funcName',
+            bindings: {
+                httpTrigger433d175fc9: expectedHttpTrigger,
+                $return: expectedHttpOutput,
+            },
+            retryOptions: undefined,
+        });
+    });
+
+    it('http trigger, storage output', () => {
+        const result = toCoreFunctionMetadata('funcName', {
+            handler,
+            trigger: trigger.http({}),
+            return: output.http({}),
+            extraOutputs: [
+                output.storageQueue({
+                    queueName: 'e2e-test-queue-trigger',
+                    connection: 'e2eTest_storage',
+                }),
+            ],
+        });
+        expect(result).to.deep.equal({
+            name: 'funcName',
+            bindings: {
+                httpTrigger433d175fc9: expectedHttpTrigger,
+                $return: expectedHttpOutput,
+                queueOutput8b95495f3d: expectedQueueOutput1,
+            },
+            retryOptions: undefined,
+        });
+    });
+
+    it('http trigger, multiple storage output', () => {
+        const result = toCoreFunctionMetadata('funcName', {
+            handler,
+            trigger: trigger.http({}),
+            return: output.http({}),
+            extraOutputs: [
+                output.storageQueue({
+                    queueName: 'e2e-test-queue-trigger',
+                    connection: 'e2eTest_storage',
+                }),
+                output.storageQueue({
+                    queueName: 'e2e-test-queue-trigger2',
+                    connection: 'e2eTest_storage',
+                }),
+            ],
+        });
+        expect(result).to.deep.equal({
+            name: 'funcName',
+            bindings: {
+                httpTrigger433d175fc9: expectedHttpTrigger,
+                $return: expectedHttpOutput,
+                queueOutput8b95495f3d: expectedQueueOutput1,
+                queueOutput7ab7ce64ad: {
+                    queueName: 'e2e-test-queue-trigger2',
+                    connection: 'e2eTest_storage',
+                    type: 'queue',
+                    name: 'queueOutput7ab7ce64ad',
+                    direction: 'out',
+                },
+            },
+            retryOptions: undefined,
+        });
+    });
+
+    it('http trigger, duplicate storage output', () => {
+        expect(() => {
+            toCoreFunctionMetadata('funcName', {
+                handler,
+                trigger: trigger.http({}),
+                return: output.http({}),
+                extraOutputs: [
+                    output.storageQueue({
+                        queueName: 'e2e-test-queue-trigger',
+                        connection: 'e2eTest_storage',
+                    }),
+                    output.storageQueue({
+                        queueName: 'e2e-test-queue-trigger',
+                        connection: 'e2eTest_storage',
+                    }),
+                ],
+            });
+        }).to.throw(/duplicate bindings found/i);
+    });
+
+    it('http trigger, duplicate storage output with name workaround', () => {
+        const result = toCoreFunctionMetadata('funcName', {
+            handler,
+            trigger: trigger.http({}),
+            return: output.http({}),
+            extraOutputs: [
+                output.storageQueue({
+                    // eslint-disable-next-line @typescript-eslint/ban-ts-comment
+                    // @ts-ignore
+                    name: 'notADupe',
+                    queueName: 'e2e-test-queue-trigger',
+                    connection: 'e2eTest_storage',
+                }),
+                output.storageQueue({
+                    queueName: 'e2e-test-queue-trigger',
+                    connection: 'e2eTest_storage',
+                }),
+            ],
+        });
+
+        expect(result).to.deep.equal({
+            name: 'funcName',
+            bindings: {
+                httpTrigger433d175fc9: expectedHttpTrigger,
+                $return: expectedHttpOutput,
+                queueOutput8b95495f3d: expectedQueueOutput1,
+                notADupe: {
+                    queueName: 'e2e-test-queue-trigger',
+                    connection: 'e2eTest_storage',
+                    type: 'queue',
+                    name: 'notADupe',
+                    direction: 'out',
+                },
+            },
+            retryOptions: undefined,
+        });
+    });
+});