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

Exception handling middleware fails for void return type #1619

Closed
Alan-Hinton opened this issue Jun 13, 2023 · 3 comments · Fixed by #2579
Closed

Exception handling middleware fails for void return type #1619

Alan-Hinton opened this issue Jun 13, 2023 · 3 comments · Fixed by #2579
Assignees
Labels
area: http Items related to experience improvements for HTTP triggers

Comments

@Alan-Hinton
Copy link

Alan-Hinton commented Jun 13, 2023

See #936, which was closed despite not being fixed.

If you use the sample exception handling middleware and have an http triggered functions with void or bare Task return type that throws an exception the request results in a 200 response and the whole http response is returned as json.

I have created a minimal example which shows the error, as well as showing how returning a different return type works fine.

i.e. the request to the void function returns the wrong status code and response

C:\Code>curl "http://localhost:7071/api/throw-exception" -H "accept: */*" -d "" -v
*   Trying 127.0.0.1:7071...
* Connected to localhost (127.0.0.1) port 7071 (#0)
> POST /api/throw-exception HTTP/1.1
> Host: localhost:7071
> User-Agent: curl/7.83.1
> accept: */*
> Content-Length: 0
> Content-Type: application/x-www-form-urlencoded
>
* Mark bundle as not supporting multiuse
< HTTP/1.1 200 OK
< Content-Length: 208
< Content-Type: application/json; charset=utf-8
< Date: Wed, 19 Oct 2022 19:33:06 GMT
< Server: Kestrel
<
{"method":"","query":{},"statusCode":"500","headers":{"Content-Type":"application/json; charset=utf-8"},"enableContentNegotiation":false,"cookies":[],"body":"eyJGb29TdGF0dXMiOiJJbnZvY2F0aW9uIGZhaWxlZCEifQ=="}* Connection #0 to host localhost left intact

whereas the endpoint that has a non-void return type gives the correct 500 response

C:\Code>curl "http://localhost:7071/api/throw-exception-and-return/true" -H "accept: */*" -d "" -v
*   Trying 127.0.0.1:7071...
* Connected to localhost (127.0.0.1) port 7071 (#0)
> POST /api/throw-exception-and-return/true HTTP/1.1
> Host: localhost:7071
> User-Agent: curl/7.83.1
> accept: */*
> Content-Length: 0
> Content-Type: application/x-www-form-urlencoded
>
* Mark bundle as not supporting multiuse
< HTTP/1.1 500 Internal Server Error
< Content-Type: application/json; charset=utf-8
< Date: Wed, 19 Oct 2022 19:35:16 GMT
< Server: Kestrel
< Transfer-Encoding: chunked
<
{"FooStatus":"Invocation failed!"}* Connection #0 to host localhost left intact

I have tried fiddling with the middleware to get this working, but there doesn't seem to be a way. So I think this is a bug in the underlying framework. There doesn't seem to be any reason I shouldn't be able to have a request with a void return type.

@ghost ghost assigned brettsam Jun 13, 2023
@liliankasem liliankasem added needs-investigation potential-bug Items opened using the bug report template, not yet triaged and confirmed as a bug labels Apr 18, 2024
@fabiocav fabiocav assigned kshyju and unassigned brettsam Jun 19, 2024
@kshyju
Copy link
Member

kshyju commented Jul 3, 2024

Notes from investigation:

  • api/throw-exception-and-return/true -> throws exception from function. Method has a POCO return type.
  • api/throw-exception -> throws exception from function code. Method has void as return type for http trigger.

In both cases (api/throw-exception-and-return/true & api/throw-exception), the worker is returning the same invocation response back to host. The host code which handles the invocation response is executing differently for these 2 cases.

The function metadata generated for the 2 functions are as below. note that the "throw-exception" function is missing the $return binding.

public Task<ImmutableArray<IFunctionMetadata>> GetFunctionMetadataAsync(string directory)
{
    var metadataList = new List<IFunctionMetadata>();
    var Function0RawBindings = new List<string>();
    Function0RawBindings.Add(@"{""name"":""req"",""type"":""httpTrigger"",""direction"":""In"",""authLevel"":""Anonymous"",""methods"":[""get""],""route"":""throw-exception""}");

    var Function0 = new DefaultFunctionMetadata
    {
        Language = "dotnet-isolated",
        Name = "ThrowException",
        EntryPoint = "MiddlewareStatusCode.ExceptionController.ThrowException",
        RawBindings = Function0RawBindings,
        ScriptFile = "MiddlewareStatusCode.dll"
    };
    metadataList.Add(Function0);
    var Function1RawBindings = new List<string>();
    Function1RawBindings.Add(@"{""name"":""req"",""type"":""httpTrigger"",""direction"":""In"",""authLevel"":""Anonymous"",""methods"":[""get""],""route"":""throw-exception-and-return/{returnUsefullData}""}");
    Function1RawBindings.Add(@"{""name"":""$return"",""type"":""http"",""direction"":""Out""}");

    var Function1 = new DefaultFunctionMetadata
    {
        Language = "dotnet-isolated",
        Name = "ThrowExceptionAndReturn",
        EntryPoint = "MiddlewareStatusCode.ExceptionController.ThrowExceptionAndReturn",
        RawBindings = Function1RawBindings,
        ScriptFile = "MiddlewareStatusCode.dll"
    };
    metadataList.Add(Function1);

    return Task.FromResult(metadataList.ToImmutableArray());
}

The WorkerFunctionInvoker.BindOutputAsync relies on the output bindings from the metadata to handle the response(output binding data to be specific) after invocation.

https://github.com/Azure/azure-functions-host/blob/94bd90c9ef10c6be61ff9232c189f5721a512152/src/WebJobs.Script/Description/Workers/WorkerFunctionInvoker.cs#L161-L187

For the "throw-exception-and-return" function, there is one output binding which causes the HttpBinding.BindAsync method to be invoked. For throw-exception case, since there are no output bindings, the FunctionBinding.BindAsync is not engaged at all, causing the response to be eventually serialized and sent out with a 200 status code.

Next step is to discuss how /where to handle this.

Option1

Update metadata generation to create the $return binding regardless of functions return type, if the function is Http trigger. What are the impacts of this change?

Option2

Update host code to use a fallback HttpBinding.BindAsync For http trigger invocation, this could be the HttpBinding binder.

@fabiocav
Copy link
Member

fabiocav commented Jul 3, 2024

Discussed this in triage. We'll move forward with option 1 and keep this issue open to track the work.

@fabiocav fabiocav added area: http Items related to experience improvements for HTTP triggers and removed needs-investigation potential-bug Items opened using the bug report template, not yet triaged and confirmed as a bug labels Jul 3, 2024
@kshyju
Copy link
Member

kshyju commented Jul 11, 2024

@Alan-Hinton

Version 1.17.4 of Worker.Sdk includes the fix for the issue. Please upgrade your package to this version, and feel free to reach out if you encounter any further issues.

Microsoft.Azure.Functions.Worker.Sdk 1.17.4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: http Items related to experience improvements for HTTP triggers
Projects
None yet
5 participants