C# runtime can break based on parameter name

[davidebbo]

The following function runs fine:

using System;
using System.Net;
using System.Net.Http;
using System.Threading.Tasks;
using Microsoft.Azure.WebJobs.Host;

public static Task<HttpResponseMessage> Run(HttpRequestMessage input, TraceWriter log)
{
    HttpResponseMessage res = new HttpResponseMessage(HttpStatusCode.OK)
    {
        Content = new StringContent("Please pass a name on the query string")
    };

    return Task.FromResult(res);
}

Now replace HttpRequestMessage input with HttpRequestMessage req, and it blows up at runtime with a generic 500 error.

So really two issues:

1. Why does the parameter name matter?
2. Can we improve error handling?

[mathewc]

Yes, currently he's matching parameters in metadata to those in code via name. Only an issue for C#. When you don't explicitly name your param in function.json it defaults to 'input'. If you added name: 'req' to your binding it would work.

This is just a loose end. C# parameter matching should probably be positional. For all other language types other than C# it makes sense to specify names in config, but for C# the names come from the code. I had this on my list to go look into / fix after the C# review.

[davidebbo]

I see. That feels a little too magical :) And the lack of error handling made it super hard to diagnose.

Note that our template is broken out of the box: https://github.com/Azure/azure-webjobs-sdk-templates/tree/master/Templates/HttpTrigger-CSharp (I'll fix it).

[mathewc]

I don't think it's broken - the default will be 'input' in metadata and that matches. The same one in this repo samples works as is as well.

[davidebbo]

Only because I just fixed it :)

[fabiocav]

👍

[davidebbo]

@fabiocav: why does that break at runtime, but not at compile time in the designer? That would help catch this more easily. e.g. "your function must have an input param".

[fabiocav]

Yes, we could turn this into a compile time error, which would lead to a much better experience. I'd like to chat with @mathewc to get some of his ideas. I have concerns about making it positional, but not having a parameter (e.g. "input") should be ok. If I don't need to actually consume that input, I could omit it. The issue would be the other way around, where I have parameters we wouldn't know how to populate at runtime. But the argument parameters from the bindings (in addition to the bindings themselves) would play a role there as well.

[davidebbo]

My take is that with good error handling, there is no need to make it positional. We could make it very clear what's wrong, by looking at both what they declared and what we have available. In the case above, the message could be:

you have declared a parameter named req, but you don't have any bindings by that name. The available binding names are: 'input`.

[mathewc]

Yes it would be good if we don't have to treat C# specially in this regard. It does seem strange to me that you'll have to name your parameters both in metadata, and in code, and keep them in sync. However maybe that's the safest way of associating metadata.

[davidebbo]

@mathewc but isn't that exactly what you're advocating in #50? If you have to dig out the params from the JS context by name, you basically have to name your parameters both in metadata, and in code, and keep them in sync. No?