Improve error messages for implicit and explicit FromBody attribute in Minimal API #35086 #35188

rafikiassumani-msft · 2021-08-09T19:03:54Z

Improve error messages for implicit and explicit FromBody attribute in Minimal API #35086

PR Description

This a draft PR to start the conversation on a possible solution for error messages when :

The Minimal MapPost/Put/Patch/etc.. has two body parameters either explicitly or implicitly. This would address a case where one of the body parameter may be a service that the user forgot to register with the DI container and binding fails to resolve.

Example:

  app.MapPost("/car", ([FromBody] SampleDTO sampleDTO, IUserRepo userRepo) => $"{sampleDTO.Name}");

In the example above, the code changes will keep track of all the request parameters and provide an error message telling the user which parameters we could successfully infer which ones fail. It also adds the parameter source (From Query String, Route, Services, etc.)

  Unhandled exception. System.InvalidOperationException: Failure to infer one or more parameters. Below is the list of parameters we found:

Parameter          Source

sampleDTO  Body Attribute
userRepo We failed to infer this parameter. Did you forget to inject it as a Service?

   at Microsoft.AspNetCore.Http.RequestDelegateFactory.CreateArguments(ParameterInfo[] parameters, FactoryContext factoryContext) in C:\Users\rassumani\bench\aspnetcore\src\Http\Http.Extensions\src\RequestDelegateFactory.cs:line 209
   at Microsoft.AspNetCore.Http.RequestDelegateFactory.CreateTargetableRequestDelegate(MethodInfo methodInfo, RequestDelegateFactoryOptions options, Expression targetExpression) in C:\Users\rassumani\bench\aspnetcore\src\Http\Http.Extensions\src\RequestDelegateFactory.cs:line 170
   at Microsoft.AspNetCore.Http.RequestDelegateFactory.Create(Delegate action, RequestDelegateFactoryOptions options) in C:\Users\rassumani\bench\aspnetcore\src\Http\Http.Extensions\src\RequestDelegateFactory.cs:line 89
   at Microsoft.AspNetCore.Builder.MinimalActionEndpointRouteBuilderExtensions.Map(IEndpointRouteBuilder endpoints, RoutePattern pattern, Delegate action) in C:\Users\rassumani\bench\aspnetcore\src\Http\Routing\src\Builder\MinimalActionEndpointRouteBuilderExtensions.cs:line 173
   at Microsoft.AspNetCore.Builder.MinimalActionEndpointRouteBuilderExtensions.MapMethods(IEndpointRouteBuilder endpoints, String pattern, IEnumerable`1 httpMethods, Delegate action) in C:\Users\rassumani\bench\aspnetcore\src\Http\Routing\src\Builder\MinimalActionEndpointRouteBuilderExtensions.cs:line 109
   at Microsoft.AspNetCore.Builder.MinimalActionEndpointRouteBuilderExtensions.MapPost(IEndpointRouteBuilder endpoints, String pattern, Delegate action) in C:\Users\rassumani\bench\aspnetcore\src\Http\Routing\src\Builder\MinimalActionEndpointRouteBuilderExtensions.cs:line 54

There is No content type and the content length is zero, however, there is still an implicit FromBody parameter that we need to bind like in the case below:

 app.MapGet("/test/{id}", (int id, SampleDTO sampleDTO) => $"id is {id}  - {sampleDTO.Name}");

We will read the body httpContext.Request.ReadFromJsonAsync(bodyType) and throw an exception in CreateContentTypeError method.

Exception:

  System.InvalidOperationException: We failed to infer or bind 'SampleDTO' parameter. Was this a Service parameter that you forgot to register ?
   at Microsoft.AspNetCore.Http.HttpRequestJsonExtensions.ReadFromJsonAsync(HttpRequest request, Type type, JsonSerializerOptions options, CancellationToken cancellationToken) in C:\Users\rassumani\bench\aspnetcore\src\Http\Http.Extensions\src\HttpRequestJsonExtensions.cs:line 129
   at Microsoft.AspNetCore.Http.RequestDelegateFactory.<>c__DisplayClass42_0.<<HandleRequestBodyAndCompileRequestDelegate>b__0>d.MoveNext() in C:\Users\rassumani\bench\aspnetcore\src\Http\Http.Extensions\src\RequestDelegateFactory.cs:line 517
--- End of stack trace from previous location ---
   at Microsoft.AspNetCore.Routing.EndpointMiddleware.<Invoke>g__AwaitRequestTask|6_0(Endpoint endpoint, Task requestTask, ILogger logger) in C:\Users\rassumani\bench\aspnetcore\src\Http\Routing\src\EndpointMiddleware.cs:line 79
   at Microsoft.AspNetCore.Diagnostics.DeveloperExceptionPageMiddleware.Invoke(HttpContext context) in C:\Users\rassumani\bench\aspnetcore\src\Middleware\Diagnostics\src\DeveloperExceptionPage\DeveloperExceptionPageMiddleware.cs:line 94

Fixes #35086

…n Minimal API

src/Http/Http.Extensions/src/HttpRequestJsonExtensions.cs

src/Http/Http.Extensions/src/RequestDelegateFactory.cs

halter73 · 2021-08-09T19:21:37Z

I think we should split this into two PRs. One for throwing for multiple from-body parameters during Create() and another for throwing for not finding a body when one is expected during the request.

src/Http/Http.Extensions/src/HttpRequestJsonExtensions.cs

src/Http/Http.Extensions/src/RequestDelegateFactory.cs

halter73 · 2021-08-17T00:26:02Z

src/Http/Http.Extensions/src/RequestDelegateFactory.cs

+                if (parameterName is not null && factoryContext.TrackedParameters.ContainsKey(parameterName))
+                {
+                    factoryContext.TrackedParameters.Remove(parameterName);
+                    factoryContext.TrackedParameters.Add(parameter.Name!, "We failed to infer this parameter. Did you forget to inject it as a Service in the DI container?");


Suggested change

factoryContext.TrackedParameters.Add(parameter.Name!, "We failed to infer this parameter. Did you forget to inject it as a Service in the DI container?");

factoryContext.TrackedParameters.Add(parameter.Name!, "UNKNOWN");

I think the following is a bit hard to read:

Unhandled exception. System.InvalidOperationException: Failure to infer one or more parameters. Below is the list of parameters we found: Parameter Source sampleDTO Body Attribute userRepo We failed to infer this parameter. Did you forget to inject it as a Service? at Microsoft.AspNetCore.Http.RequestDelegateFactory.CreateArguments(ParameterInfo[] parameters, FactoryContext factoryContext) in C:\Users\rassumani\bench\aspnetcore\src\Http\Http.Extensions\src\RequestDelegateFactory.cs:line 209 at Microsoft.AspNetCore.Http.RequestDelegateFactory.CreateTargetableRequestDelegate(MethodInfo methodInfo, RequestDelegateFactoryOptions options, Expression targetExpression) in C:\Users\rassumani\bench\aspnetcore\src\Http\Http.Extensions\src\RequestDelegateFactory.cs:line 170 ...

This would be a bit better:

Unhandled exception. System.InvalidOperationException: Failure to infer one or more parameters. Below is the list of parameters we found: Parameter | Source ----------+------------------ sampleDTO | Body (Attribute) userRepo | UNKOWN Did you forget to inject the "UNKNOWN" parameters as a Service? at Microsoft.AspNetCore.Http.RequestDelegateFactory.CreateArguments(ParameterInfo[] parameters, FactoryContext factoryContext) in C:\Users\rassumani\bench\aspnetcore\src\Http\Http.Extensions\src\RequestDelegateFactory.cs:line 209 at Microsoft.AspNetCore.Http.RequestDelegateFactory.CreateTargetableRequestDelegate(MethodInfo methodInfo, RequestDelegateFactoryOptions options, Expression targetExpression) in C:\Users\rassumani\bench\aspnetcore\src\Http\Http.Extensions\src\RequestDelegateFactory.cs:line 170 ...

Adding the | and - would require more than just the suggested fix of course but I think it will help. Can you show what this looks like on the developer exception page?

Also, do we get the same "Failure to infer one or more parameters." error when all the parameters have explicit attributes? If there actually are multiple [FromBody] attributes we should probably just continue saying that's not allowed instead of saying we couldn't infer the source.

Do we have a logger formatter or console formatter in aspnetcore that I can use ?

I am hoping an analyzer will prevent the double explicit [FromBody] right away.

New formatting. Not perfect, visually readable.

Unhandled exception. System.InvalidOperationException: Failure to infer one or more parameters. Below is the list of parameters that we found: Parameter |Source --------------------------------------------------------------------------------- id | Route (Inferred) dto | Body (Attribute) dto1 | UNKNOWN repo | Service (Attribute) repo1 | UNKNOWN Did you forget to inject the "UNKNOWN" parameters as a Service? at Microsoft.AspNetCore.Http.RequestDelegateFactory.CreateArguments(ParameterInfo[] parameters, FactoryContext factoryContext) in C:\Users\rassumani\bench\aspnetcore\src\Http\Http.Extensions\src\RequestDelegateFactory.cs:line 196 at Microsoft.AspNetCore.Http.RequestDelegateFactory.CreateTargetableRequestDelegate(MethodInfo methodInfo, RequestDelegateFactoryOptions options, Expression targetExpression) in C:\Users\rassumani\bench\aspnetcore\src\Http\Http.Extensions\src\RequestDelegateFactory.cs:line 165 at Microsoft.AspNetCore.Http.RequestDelegateFactory.Create(Delegate action, RequestDelegateFactoryOptions options) in C:\Users\rassumani\bench\aspnetcore\src\Http\Http.Extensions\src\RequestDelegateFactory.cs:line 84

"Did you mean to register the "UNKNOWN" parameters as a Service?"

Unhandled exception. System.InvalidOperationException: Failure to infer one or more parameters. Below is the list of parameters that we found: Parameter |Source --------------------------------------------------------------------------------- id | Route (Inferred) dto | Body (Attribute) dto1 | UNKNOWN repo | Service (Attribute) repo1 | UNKNOWN Did you mean to register the "UNKNOWN" parameters as a Service? at Microsoft.AspNetCore.Http.RequestDelegateFactory.CreateArguments(ParameterInfo[] parameters, FactoryContext factoryContext) in C:\Users\rassumani\bench\aspnetcore\src\Http\Http.Extensions\src\RequestDelegateFactory.cs:line 196 at Microsoft.AspNetCore.Http.RequestDelegateFactory.CreateTargetableRequestDelegate(MethodInfo methodInfo, RequestDelegateFactoryOptions options, Expression targetExpression) in C:\Users\rassumani\bench\aspnetcore\src\Http\Http.Extensions\src\RequestDelegateFactory.cs:line 165 at Microsoft.AspNetCore.Http.RequestDelegateFactory.Create(Delegate action, RequestDelegateFactoryOptions options) in C:\Users\rassumani\bench\aspnetcore\src\Http\Http.Extensions\src\RequestDelegateFactory.cs:line 84

src/Http/Http.Extensions/src/RequestDelegateFactory.cs

captainsafia · 2021-08-17T02:48:39Z

src/Http/Http.Extensions/src/RequestDelegateFactory.cs

@@ -203,6 +217,7 @@ private static Expression CreateArgument(ParameterInfo parameter, FactoryContext

            if (parameterCustomAttributes.OfType<IFromRouteMetadata>().FirstOrDefault() is { } routeAttribute)
            {
+                factoryContext.TrackedParameters.Add(parameter.Name, RequestDelegateFactoryConstants.RouteAttribue);


Great call to add a separate class for all the constants we're introducing here.

src/Http/Http.Extensions/src/HttpRequestJsonExtensions.cs

davidfowl · 2021-08-17T18:58:10Z

src/Http/Http.Extensions/src/RequestDelegateFactory.cs

@@ -189,6 +190,13 @@ private static Expression[] CreateArguments(ParameterInfo[]? parameters, Factory
                args[i] = CreateArgument(parameters[i], factoryContext);
            }

+            if(factoryContext.HasMultipleBodyParameters)


Super nit:

Suggested change

if(factoryContext.HasMultipleBodyParameters)

if (factoryContext.HasMultipleBodyParameters)

src/Http/Http.Extensions/src/RequestDelegateFactory.cs

src/Http/Http.Extensions/src/HttpRequestJsonExtensions.cs

davidfowl · 2021-08-17T19:53:34Z

src/Http/Http.Extensions/src/RequestDelegateFactory.cs

@@ -254,6 +267,7 @@ private static Expression CreateArgument(ParameterInfo parameter, FactoryContext
                // when RDF.Create is manually invoked.
                if (factoryContext.RouteParameters is { } routeParams)
                {
+                    factoryContext.TrackedParameters.Add(parameter.Name, RequestDelegateFactoryConstants.RouteParameter);


This isn't correct anymore. It's either route or query or either.

rafikiassumani-msft added 3 commits August 9, 2021 08:54

Improve error messages for implicit and explicit FromBody attribute i…

752ff7d

…n Minimal API

Add error message for mapget when user forget to register service

ecdb0fb

resolve merge conflicts

18f174c

rafikiassumani-msft requested review from BrennanConroy and Tratcher as code owners August 9, 2021 19:03

ghost added the area-runtime label Aug 9, 2021

rafikiassumani-msft requested review from davidfowl and halter73 August 9, 2021 19:04

rafikiassumani-msft added feature-minimal-actions Controller-like actions for endpoint routing old-area-web-frameworks-do-not-use *DEPRECATED* This label is deprecated in favor of the area-mvc and area-minimal labels and removed area-runtime labels Aug 9, 2021

halter73 reviewed Aug 9, 2021

View reviewed changes

src/Http/Http.Extensions/src/HttpRequestJsonExtensions.cs Outdated Show resolved Hide resolved

halter73 reviewed Aug 9, 2021

View reviewed changes

src/Http/Http.Extensions/src/RequestDelegateFactory.cs Outdated Show resolved Hide resolved

add suggested changes

47a1e90

rafikiassumani-msft marked this pull request as draft August 9, 2021 19:22

rafikiassumani-msft changed the title ~~Draft: Improve error messages for implicit and explicit FromBody attribute in Minimal API #35086~~ Improve error messages for implicit and explicit FromBody attribute in Minimal API #35086 Aug 9, 2021

rafikiassumani-msft added 2 commits August 9, 2021 13:46

replace null with typeof(Tvalue)

f834444

multiple frombody

45eeb28

rafikiassumani-msft marked this pull request as ready for review August 16, 2021 18:26

rafikiassumani-msft added 2 commits August 16, 2021 11:48

merge conflicts

72b059c

resolve more merge issues

98217cb

halter73 reviewed Aug 17, 2021

View reviewed changes

src/Http/Http.Extensions/src/HttpRequestJsonExtensions.cs Outdated Show resolved Hide resolved

halter73 reviewed Aug 17, 2021

View reviewed changes

src/Http/Http.Extensions/src/HttpRequestJsonExtensions.cs Outdated Show resolved Hide resolved

halter73 reviewed Aug 17, 2021

View reviewed changes

src/Http/Http.Extensions/src/RequestDelegateFactory.cs Outdated Show resolved Hide resolved

halter73 reviewed Aug 17, 2021

View reviewed changes

src/Http/Http.Extensions/src/RequestDelegateFactory.cs Show resolved Hide resolved

halter73 reviewed Aug 17, 2021

View reviewed changes

src/Http/Http.Extensions/src/RequestDelegateFactory.cs Outdated Show resolved Hide resolved

halter73 reviewed Aug 17, 2021

View reviewed changes

src/Http/Http.Extensions/src/RequestDelegateFactory.cs Outdated Show resolved Hide resolved

captainsafia reviewed Aug 17, 2021

View reviewed changes

src/Http/Http.Extensions/src/RequestDelegateFactory.cs Outdated Show resolved Hide resolved

captainsafia reviewed Aug 17, 2021

View reviewed changes

rafikiassumani-msft added 2 commits August 16, 2021 19:50

address pr suggestions

776b041

address more PR comments

8e3d540

davidfowl reviewed Aug 17, 2021

View reviewed changes

src/Http/Http.Extensions/src/HttpRequestJsonExtensions.cs Outdated Show resolved Hide resolved

davidfowl reviewed Aug 17, 2021

View reviewed changes

src/Http/Http.Extensions/src/RequestDelegateFactory.cs Show resolved Hide resolved

davidfowl reviewed Aug 17, 2021

View reviewed changes

src/Http/Http.Extensions/src/RequestDelegateFactory.cs Show resolved Hide resolved

davidfowl reviewed Aug 17, 2021

View reviewed changes

src/Http/Http.Extensions/src/RequestDelegateFactory.cs Outdated Show resolved Hide resolved

davidfowl reviewed Aug 17, 2021

View reviewed changes

src/Http/Http.Extensions/src/HttpRequestJsonExtensions.cs Outdated Show resolved Hide resolved

clean up

cdceb05

davidfowl reviewed Aug 17, 2021

View reviewed changes

reset HttpRequestJsonExtensions.cs

d8253c6

davidfowl approved these changes Aug 17, 2021

View reviewed changes

rafikiassumani-msft merged commit cddec67 into dotnet:main Aug 17, 2021

ghost added this to the 6.0-rc1 milestone Aug 17, 2021

amcasey added the area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc label Jun 2, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve error messages for implicit and explicit FromBody attribute in Minimal API #35086 #35188

Improve error messages for implicit and explicit FromBody attribute in Minimal API #35086 #35188

rafikiassumani-msft commented Aug 9, 2021 •

edited

Loading

halter73 commented Aug 9, 2021

halter73 Aug 17, 2021

halter73 Aug 17, 2021

rafikiassumani-msft Aug 17, 2021

rafikiassumani-msft Aug 17, 2021

rafikiassumani-msft Aug 17, 2021 •

edited

Loading

davidfowl Aug 17, 2021

rafikiassumani-msft Aug 17, 2021

captainsafia Aug 17, 2021

davidfowl Aug 17, 2021

davidfowl Aug 17, 2021

	factoryContext.TrackedParameters.Add(parameter.Name!, "We failed to infer this parameter. Did you forget to inject it as a Service in the DI container?");
	factoryContext.TrackedParameters.Add(parameter.Name!, "UNKNOWN");

	if(factoryContext.HasMultipleBodyParameters)
	if (factoryContext.HasMultipleBodyParameters)

Improve error messages for implicit and explicit FromBody attribute in Minimal API #35086 #35188

Improve error messages for implicit and explicit FromBody attribute in Minimal API #35086 #35188

Conversation

rafikiassumani-msft commented Aug 9, 2021 • edited Loading

halter73 commented Aug 9, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rafikiassumani-msft Aug 17, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rafikiassumani-msft commented Aug 9, 2021 •

edited

Loading

rafikiassumani-msft Aug 17, 2021 •

edited

Loading