Feature: allow developers to inject the MethodInfo as a Property

[james-s-tayler]

What kind of change does this PR introduce?
A feature to make the MethodInfo of the method on the Refit client interface invoked available to handlers via the HttpRequestMessage.Options / HttpRequestMessage.Properties.

This behavior is opt-in via the RefitSettings

services.AddRefitClient<ISomeAPI>(provider => new RefitSettings
            {
                InjectMethodInfoAsProperty = true
            })
        .ConfigureHttpClient(c => c.BaseAddress = new Uri("https://api.example.com"));

What is the current behavior?
People either can't do what they want or are implementing workarounds by polluting their interfaces.

There are three existing PRs for this:

• Property provider factory #1180
• Add MethodInfo property #1219
• Make the RestMethodInfo available in the request options #1317

And a couple of issues:

• feature: Add method name to HttpRequestMessage Properties #1150
• feature: Allow decoration of interface methods to be used by DelegatingHandlers #1156

What is the new behavior?
New behavior is RefitSettings.InjectMethodInfoAsProperty defaults to false so Refit as a library remains (mostly) unopinionated about the contents of HttpRequestMessage.Options/HttpRequestMessage.Properties, however if set to true it will inject the MethodInfo into HttpRequestMessage.Options/HttpRequestMessage.Properties under the key Refit.MethodInfo.

What might this PR break?
Nothing, the behavior is opt-in, so it has to explicitly be enabled giving the developer the chance to consider if there might be any implications for their codebase if the full MethodInfo were to be populated into and to put any mitigations necessary should they require it if they do wish to take advantage of the functionality.

I fixed up my benchmarks PR #1175 and benchmarked it to compare and found no differences in either execution time or allocations when injecting the MethodInfo.

net6.0 TaskApiResponseT baseline

Method	HttpStatusCode	ModelCount	Verb	Serializer	Mean	Error	StdDev	Gen 0	Gen 1	Gen 2	Allocated
TaskApiResponseT_Async	OK	10	Get	SystemTextJson	19.754 us	0.1578 us	0.1232 us	3.5706	-	-	11 KB
TaskApiResponseT_Async	OK	10	Get	NewtonsoftJson	28.393 us	0.1648 us	0.1541 us	5.5237	-	-	17 KB
TaskApiResponseT_Async	OK	10	Post	SystemTextJson	20.774 us	0.2361 us	0.2208 us	3.9978	-	-	12 KB
TaskApiResponseT_Async	OK	10	Post	NewtonsoftJson	42.334 us	0.3036 us	0.2840 us	9.7046	-	-	30 KB
TaskApiResponseT_Async	InternalServerError	10	Get	SystemTextJson	7.796 us	0.0695 us	0.0616 us	4.5624	-	-	14 KB
TaskApiResponseT_Async	InternalServerError	10	Get	NewtonsoftJson	7.736 us	0.0443 us	0.0415 us	4.5624	-	-	14 KB
TaskApiResponseT_Async	InternalServerError	10	Post	SystemTextJson	9.004 us	0.0943 us	0.0836 us	5.0049	-	-	15 KB
TaskApiResponseT_Async	InternalServerError	10	Post	NewtonsoftJson	21.806 us	0.1381 us	0.1292 us	8.7280	-	-	27 KB

net6.0 TaskApiResponseT with MethodInfo added to HttpRequestMessage.Options

Method	HttpStatusCode	ModelCount	Verb	Serializer	Mean	Error	StdDev	Gen 0	Gen 1	Gen 2	Allocated
TaskApiResponseT_Async	OK	10	Get	SystemTextJson	31.330 us	3.8873 us	11.4617 us	3.5400	-	-	11 KB
TaskApiResponseT_Async	OK	10	Get	NewtonsoftJson	28.384 us	0.1024 us	0.0855 us	5.5237	-	-	17 KB
TaskApiResponseT_Async	OK	10	Post	SystemTextJson	20.518 us	0.1655 us	0.1548 us	3.9978	-	-	12 KB
TaskApiResponseT_Async	OK	10	Post	NewtonsoftJson	43.518 us	0.2674 us	0.2502 us	9.7046	-	-	30 KB
TaskApiResponseT_Async	InternalServerError	10	Get	SystemTextJson	7.778 us	0.0678 us	0.0634 us	4.5624	-	-	14 KB
TaskApiResponseT_Async	InternalServerError	10	Get	NewtonsoftJson	7.684 us	0.1098 us	0.1027 us	4.5624	-	-	14 KB
TaskApiResponseT_Async	InternalServerError	10	Post	SystemTextJson	8.744 us	0.0516 us	0.0482 us	5.0049	-	-	15 KB
TaskApiResponseT_Async	InternalServerError	10	Post	NewtonsoftJson	21.977 us	0.2127 us	0.1989 us	8.7280	-	-	27 KB

Please check if the PR fulfills these requirements

• Tests for the changes have been added (for bug fixes / features)
• Docs have been added / updated (for bug fixes / features)

Other information:
I know previously it has been stated that it would be acceptable if we store the method name as a string, but I for one just don't feel comfortable relying on something as critical as an integration point between two systems containing some "stringly typed" behavior when it could be strongly typed. Once someone introduces an overloaded method then all bets are off and it wouldn't surprise me to see that cause a potentially nasty production incident for someone somewhere, and/or future support issues.

I believe this implementation is a good half-way point between simplicity, safety, and usability and sincerely hope we can get this merged.

[gabrielmaldi]

I would love to have this to be able to override HttpClient's timeout on a per-operation basis:

services
    .AddRefitClient()
    .ConfigureHttpClient(httpClient => httpClient.Timeout = TimeSpan.FromSeconds(30));

public class HttpMessageHandler : DelegatingHandler
{
    protected override async Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, CancellationToken cancellationToken)
    {
        var timeoutSeconds = 15; // ToDo: read configuration for operation identified by request.Options[HttpRequestMessageOptions.MethodInfoKey]
        using var timeoutTokenSource = new CancellationTokenSource(TimeSpan.FromSeconds(timeoutSeconds));
        return await base.SendAsync(request, timeoutTokenSource.Token);
    }
}

[Alguek]

merge pleeese, I really need this to look up the object type of the method

[gokhanabatay]

@james-s-tayler When dou you plan to merge this pull request?

[james-s-tayler]

@gokhanabatay you'd have to ask @clairernovotny as I don't have merge permissions.

The community has been asking for this for many years now, with about 5 different PRs open all aimed at enabling the same thing. I've sort of given up hope that it will ever be merged. Don't really get why either.

[gokhanabatay]

Hi @clairernovotny, We are trying to get executing method with some bad assumptions. I think its a good implementation but I cannot understand why its still waiting to merge. Dou you have any idea what its needed to be done to merge this pull request.

I don't know who is in charge so I ask you to discuss with community.

@james-s-tayler thank you for quick response, Does community thinks exposing whole method info is a bad idea, if so why interface type already exposed with above method. Maybe exposing current executing method hash code would be enough to get method info inside delegate handler?

[github-actions]

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.