[Bug]: Refit is losing the stacktrace due to throw ex in the client implementation

[Barsonax]

Describe the bug 🐞

When using integration tests with WebApplicationFactory it is handy to be able to see the full stack trace. Unfortunately refit's code gen generates this:

try
            {
                return await ((global::System.Threading.Tasks.Task<global::System.Net.Http.HttpResponseMessage>)______func(this.Client, ______arguments)).ConfigureAwait(false);
            }
            catch (global::System.Exception ______ex)
            {
                throw ______ex;
            }

This causes the entire stack trace of the exception to be discarded:

System.Exception : fooba
   at Refit.Implementation.Generated.CleanAspCoreApiTestsEndpointsDepartmentsIDepartmentApiClient.global::CleanAspCore.Api.Tests.Endpoints.Departments.IDepartmentApiClient.CreateDepartment(CreateDepartmentRequest createDepartmentRequest) in H:\git\CleanAspCoreWebApiTemplate\CleanAspCore.Api.Tests\obj\Debug\net8.0\InterfaceStubGeneratorV2\Refit.Generator.InterfaceStubGeneratorV2\IDepartmentApiClient.g.cs:line 96
   at CleanAspCore.Api.Tests.Endpoints.Departments.AddDepartmentsTests.CreateDepartment_IsAdded() in H:\git\CleanAspCoreWebApiTemplate\CleanAspCore.Api.Tests\Endpoints\Departments\AddDepartmentsTests.cs:line 15

When using the httpclient directly you get a much more detailed stacktrace:

System.Exception : fooba
   at CleanAspCore.Endpoints.Departments.AddDepartments.Handle(HrContext context, CreateDepartmentRequest createDepartmentRequest, CancellationToken cancellationToken) in H:\git\CleanAspCoreWebApiTemplate\CleanAspCore\Endpoints\Departments\AddDepartments.cs:line 38
   at Microsoft.AspNetCore.Http.RequestDelegateFactory.<TaskOfTToValueTaskOfObject>g__ExecuteAwaited|92_0[T](Task`1 task)
   at CleanAspCore.Common.GenericValidation.ValidationFilter`1.InvokeAsync(EndpointFilterInvocationContext context, EndpointFilterDelegate next) in H:\git\CleanAspCoreWebApiTemplate\CleanAspCore\Common\GenericValidation\ValidationFilter.cs:line 16
   at Microsoft.AspNetCore.Http.RequestDelegateFactory.<ExecuteValueTaskOfObject>g__ExecuteAwaited|129_0(ValueTask`1 valueTask, HttpContext httpContext, JsonTypeInfo`1 jsonTypeInfo)
   at Microsoft.AspNetCore.Http.RequestDelegateFactory.<>c__DisplayClass102_2.<<HandleRequestBodyAndCompileRequestDelegateForJson>b__2>d.MoveNext()
--- End of stack trace from previous location ---
   at Microsoft.AspNetCore.Routing.EndpointMiddleware.<Invoke>g__AwaitRequestTask|7_0(Endpoint endpoint, Task requestTask, ILogger logger)
   at Microsoft.AspNetCore.Authorization.AuthorizationMiddleware.Invoke(HttpContext context)
   at Microsoft.AspNetCore.Authentication.AuthenticationMiddleware.Invoke(HttpContext context)
   at Microsoft.AspNetCore.TestHost.HttpContextBuilder.<>c__DisplayClass23_0.<<SendAsync>g__RunRequestAsync|0>d.MoveNext()
--- End of stack trace from previous location ---
   at Microsoft.AspNetCore.TestHost.ClientHandler.SendAsync(HttpRequestMessage request, CancellationToken cancellationToken)
   at Microsoft.AspNetCore.Mvc.Testing.Handlers.CookieContainerHandler.SendAsync(HttpRequestMessage request, CancellationToken cancellationToken)
   at Microsoft.AspNetCore.Mvc.Testing.Handlers.RedirectHandler.SendAsync(HttpRequestMessage request, CancellationToken cancellationToken)
   at System.Net.Http.HttpClient.<SendAsync>g__Core|83_0(HttpRequestMessage request, HttpCompletionOption completionOption, CancellationTokenSource cts, Boolean disposeCts, CancellationTokenSource pendingRequestsCts, CancellationToken originalCancellationToken)
   at CleanAspCore.Api.Tests.Endpoints.Departments.AddDepartmentsTests.CreateDepartment_IsAdded() in H:\git\CleanAspCoreWebApiTemplate\CleanAspCore.Api.Tests\Endpoints\Departments\AddDepartmentsTests.cs:line 14

If this code was generated the stack trace would be preserved:

try
            {
                return await ((global::System.Threading.Tasks.Task<global::System.Net.Http.HttpResponseMessage>)______func(this.Client, ______arguments)).ConfigureAwait(false);
            }
            catch (global::System.Exception ______ex)
            {
                throw;
            }

See also https://learn.microsoft.com/en-us/dotnet/standard/exceptions/best-practices-for-exceptions#capture-and-rethrow-exceptions-properly for best practices regarding exception rethrowing

Step to reproduce

1. Use refit in ASP .NET integration test
2. Force exception to be throw on the server
3. Notice the lack of stacktrace information

Reproduction repository

https://github.com/Barsonax/CleanAspCoreWebApiTemplate/blob/main/CleanAspCore.Api.Tests/Endpoints/Departments/AddDepartmentsTests.cs

Expected behavior

The full stack trace should be available

Screenshots 🖼️

No response

IDE

Rider Windows

Operating system

Windows

Version

No response

Device

No response

Refit Version

7.1.2

Additional information ℹ️

No response

[mithileshz]

Could I pick this up? I'm looking to contribute to open source projects that I use, so seems like an easy one to get started with?

[Barsonax]

@mithileshz Iam not a contributor to refit but it indeed seems like an easy change. Not sure though if the designers of refit had an explicit reason to drop the stacktrace though. I cant think of a valid reason of why this is desired behavior and if there is then it should atleast be configurable.

[TimothyMakkison]

Not a maintainer but this seem like a good idea. I wonder why it was added. Is the try catch block necessary? Nothing happens in the catch block.

Could I pick this up? I'm looking to contribute to open source projects that I use, so seems like an easy one to get started with?

@mithileshz Seems like a good starting PR, feel free to ask me questions if you get stuck:

• Relevant code is at line 535 in InterfaceStubGenerator.Shared\InterfaceStubGenerator.cs
• Refit uses Verify to validate the generated code, this will cause the tests to fail. You can either manually fix the tests with the diff tool or use an extensions to accept all the changes. I use Verify.Support for rider (can't remember how to do it in VS2022).

If you'd like to make another change, try removing the unneeded whitespace after the constructor.

[mithileshz]

Yeah the try catch block seemed unnecessary to me too. Will create a PR later today.

[github-actions[bot]]

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