From fa8c25464fdd14f0f41b24a7b5b529929b2c33ff Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 25 Mar 2026 16:18:20 +0000 Subject: [PATCH 1/3] Initial plan From a63079e69d574201b96e1e73e48de03eccd62f63 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 25 Mar 2026 16:29:51 +0000 Subject: [PATCH 2/3] Fix sync pipeline to replicate full BuildCancellableTaskFuncForMethod behavior - Rewrite BuildGeneratedSyncFuncForMethod as a dispatcher that handles void return separately and delegates non-void to a new generic method. - Add BuildGeneratedSyncFuncForMethodGeneric that mirrors the full async pipeline: runs ExceptionFactory (skipped for HttpResponseMessage), honours IsApiResponse, uses ShouldDisposeResponse for cleanup, and surfaces DeserializationExceptionFactory on deserialization errors. - Add DeserializeContentSync handling all content types: HttpResponseMessage, HttpContent, Stream, string, and deserialised T. - Fix RestMethodInfo sync branch to properly decompose IApiResponse / ApiResponse return types, setting DeserializedResultType to the inner type just like the async Task> path does. - Add SyncVoid to ReturnTypeInfo enum and handle it in Parser.cs and Emitter.cs so void-returning sync stub methods emit a plain call instead of the invalid 'return (void)...' pattern. - Add comprehensive tests covering error responses, HttpResponseMessage, HttpContent, Stream, IApiResponse, and void sync methods. Co-authored-by: ChrisPulman <4910015+ChrisPulman@users.noreply.github.com> Agent-Logs-Url: https://github.com/reactiveui/refit/sessions/7e184030-9d18-44ec-aff4-305b2c7e973d --- InterfaceStubGenerator.Shared/Emitter.cs | 7 +- .../Models/MethodModel.cs | 3 +- InterfaceStubGenerator.Shared/Parser.cs | 2 + Refit.Tests/ExplicitInterfaceRefitTests.cs | 182 ++++++++++++++++- Refit/RequestBuilderImplementation.cs | 192 +++++++++++++++--- Refit/RestMethodInfo.cs | 22 +- 6 files changed, 373 insertions(+), 35 deletions(-) diff --git a/InterfaceStubGenerator.Shared/Emitter.cs b/InterfaceStubGenerator.Shared/Emitter.cs index f8364a2fc..e6daadbfe 100644 --- a/InterfaceStubGenerator.Shared/Emitter.cs +++ b/InterfaceStubGenerator.Shared/Emitter.cs @@ -188,6 +188,7 @@ UniqueNameBuilder uniqueNames ReturnTypeInfo.AsyncVoid => (true, "await (", ").ConfigureAwait(false)"), ReturnTypeInfo.AsyncResult => (true, "return await (", ").ConfigureAwait(false)"), ReturnTypeInfo.Return => (false, "return ", ""), + ReturnTypeInfo.SyncVoid => (false, "", ""), _ => throw new ArgumentOutOfRangeException( nameof(methodModel.ReturnTypeMetadata), methodModel.ReturnTypeMetadata, @@ -228,12 +229,16 @@ UniqueNameBuilder uniqueNames lookupName = lookupName.Substring(lastDotIndex + 1); } + var callExpression = methodModel.ReturnTypeMetadata == ReturnTypeInfo.SyncVoid + ? $"______func(this.Client, ______arguments);" + : $"{@return}({returnType})______func(this.Client, ______arguments){configureAwait};"; + source.WriteLine( $""" var ______arguments = {argumentsArrayString}; var ______func = requestBuilder.BuildRestResultFuncForMethod("{lookupName}", {parameterTypesExpression}{genericString} ); - {@return}({returnType})______func(this.Client, ______arguments){configureAwait}; + {callExpression} """ ); diff --git a/InterfaceStubGenerator.Shared/Models/MethodModel.cs b/InterfaceStubGenerator.Shared/Models/MethodModel.cs index 6f6170c10..3a8163f00 100644 --- a/InterfaceStubGenerator.Shared/Models/MethodModel.cs +++ b/InterfaceStubGenerator.Shared/Models/MethodModel.cs @@ -17,5 +17,6 @@ internal enum ReturnTypeInfo : byte { Return, AsyncVoid, - AsyncResult + AsyncResult, + SyncVoid } diff --git a/InterfaceStubGenerator.Shared/Parser.cs b/InterfaceStubGenerator.Shared/Parser.cs index ebd7e6dee..3b7cda4c2 100644 --- a/InterfaceStubGenerator.Shared/Parser.cs +++ b/InterfaceStubGenerator.Shared/Parser.cs @@ -462,6 +462,7 @@ bool isDerived { "Task" => ReturnTypeInfo.AsyncVoid, "Task`1" or "ValueTask`1" => ReturnTypeInfo.AsyncResult, + "Void" => ReturnTypeInfo.SyncVoid, _ => ReturnTypeInfo.Return, }; @@ -623,6 +624,7 @@ private static MethodModel ParseMethod(IMethodSymbol methodSymbol, bool isImplic { "Task" => ReturnTypeInfo.AsyncVoid, "Task`1" or "ValueTask`1" => ReturnTypeInfo.AsyncResult, + "Void" => ReturnTypeInfo.SyncVoid, _ => ReturnTypeInfo.Return, }; diff --git a/Refit.Tests/ExplicitInterfaceRefitTests.cs b/Refit.Tests/ExplicitInterfaceRefitTests.cs index a07d3ab96..1cc7da51b 100644 --- a/Refit.Tests/ExplicitInterfaceRefitTests.cs +++ b/Refit.Tests/ExplicitInterfaceRefitTests.cs @@ -1,4 +1,6 @@ -using System.Net.Http; +using System.Net; +using System.Net.Http; +using System.Net.Http.Headers; using System.Threading.Tasks; using Refit; using RichardSzalay.MockHttp; @@ -35,6 +37,31 @@ public interface IRemoteFoo2 : IFoo abstract int IFoo.Bar(); } + // Interfaces used to test the full sync pipeline + public interface ISyncPipelineApi + { + [Get("/resource")] + internal string GetString(); + + [Get("/resource")] + internal HttpResponseMessage GetHttpResponseMessage(); + + [Get("/resource")] + internal HttpContent GetHttpContent(); + + [Get("/resource")] + internal Stream GetStream(); + + [Get("/resource")] + internal IApiResponse GetApiResponse(); + + [Get("/resource")] + internal IApiResponse GetRawApiResponse(); + + [Get("/resource")] + internal void DoVoid(); + } + [Fact] public void DefaultInterfaceImplementation_calls_internal_refit_method() { @@ -70,4 +97,157 @@ public void Explicit_interface_member_with_refit_attribute_is_invoked() mockHttp.VerifyNoOutstandingExpectation(); } + + [Fact] + public void Sync_method_throws_ApiException_on_error_response() + { + var mockHttp = new SyncCapableMockHttpMessageHandler(); + var settings = new RefitSettings { HttpMessageHandlerFactory = () => mockHttp }; + + mockHttp + .Expect(HttpMethod.Get, "http://foo/resource") + .Respond(HttpStatusCode.NotFound); + + var fixture = RestService.For("http://foo", settings); + + var ex = Assert.Throws(() => fixture.GetString()); + Assert.Equal(HttpStatusCode.NotFound, ex.StatusCode); + + mockHttp.VerifyNoOutstandingExpectation(); + } + + [Fact] + public void Sync_method_returns_HttpResponseMessage_without_running_ExceptionFactory() + { + var mockHttp = new SyncCapableMockHttpMessageHandler(); + var settings = new RefitSettings { HttpMessageHandlerFactory = () => mockHttp }; + + mockHttp + .Expect(HttpMethod.Get, "http://foo/resource") + .Respond(HttpStatusCode.NotFound); + + var fixture = RestService.For("http://foo", settings); + + // Should not throw even for a 404 – caller owns the response + using var resp = fixture.GetHttpResponseMessage(); + Assert.Equal(HttpStatusCode.NotFound, resp.StatusCode); + + mockHttp.VerifyNoOutstandingExpectation(); + } + + [Fact] + public void Sync_method_returns_HttpContent_without_disposing_response() + { + var mockHttp = new SyncCapableMockHttpMessageHandler(); + var settings = new RefitSettings { HttpMessageHandlerFactory = () => mockHttp }; + + mockHttp + .Expect(HttpMethod.Get, "http://foo/resource") + .Respond("text/plain", "hello"); + + var fixture = RestService.For("http://foo", settings); + + var content = fixture.GetHttpContent(); + Assert.NotNull(content); + var text = content.ReadAsStringAsync().GetAwaiter().GetResult(); + Assert.Equal("hello", text); + + mockHttp.VerifyNoOutstandingExpectation(); + } + + [Fact] + public void Sync_method_returns_Stream_without_disposing_response() + { + var mockHttp = new SyncCapableMockHttpMessageHandler(); + var settings = new RefitSettings { HttpMessageHandlerFactory = () => mockHttp }; + + mockHttp + .Expect(HttpMethod.Get, "http://foo/resource") + .Respond("text/plain", "hello"); + + var fixture = RestService.For("http://foo", settings); + + using var stream = fixture.GetStream(); + Assert.NotNull(stream); + using var reader = new StreamReader(stream); + Assert.Equal("hello", reader.ReadToEnd()); + + mockHttp.VerifyNoOutstandingExpectation(); + } + + [Fact] + public void Sync_method_returns_IApiResponse_with_error_on_bad_status() + { + var mockHttp = new SyncCapableMockHttpMessageHandler(); + var settings = new RefitSettings { HttpMessageHandlerFactory = () => mockHttp }; + + mockHttp + .Expect(HttpMethod.Get, "http://foo/resource") + .Respond(HttpStatusCode.InternalServerError); + + var fixture = RestService.For("http://foo", settings); + + using var apiResp = fixture.GetApiResponse(); + Assert.False(apiResp.IsSuccessStatusCode); + Assert.NotNull(apiResp.Error); + Assert.Equal(HttpStatusCode.InternalServerError, apiResp.Error!.StatusCode); + + mockHttp.VerifyNoOutstandingExpectation(); + } + + [Fact] + public void Sync_method_returns_IApiResponse_with_content_on_success() + { + var mockHttp = new SyncCapableMockHttpMessageHandler(); + var settings = new RefitSettings { HttpMessageHandlerFactory = () => mockHttp }; + + mockHttp + .Expect(HttpMethod.Get, "http://foo/resource") + .Respond("application/json", "\"hello\""); + + var fixture = RestService.For("http://foo", settings); + + using var apiResp = fixture.GetApiResponse(); + Assert.True(apiResp.IsSuccessStatusCode); + Assert.Null(apiResp.Error); + // The string branch reads the raw stream (no JSON unwrapping), same as the async path + Assert.Equal("\"hello\"", apiResp.Content); + + mockHttp.VerifyNoOutstandingExpectation(); + } + + [Fact] + public void Sync_void_method_throws_ApiException_on_error_response() + { + var mockHttp = new SyncCapableMockHttpMessageHandler(); + var settings = new RefitSettings { HttpMessageHandlerFactory = () => mockHttp }; + + mockHttp + .Expect(HttpMethod.Get, "http://foo/resource") + .Respond(HttpStatusCode.BadRequest); + + var fixture = RestService.For("http://foo", settings); + + var ex = Assert.Throws(() => fixture.DoVoid()); + Assert.Equal(HttpStatusCode.BadRequest, ex.StatusCode); + + mockHttp.VerifyNoOutstandingExpectation(); + } + + [Fact] + public void Sync_void_method_succeeds_on_ok_response() + { + var mockHttp = new SyncCapableMockHttpMessageHandler(); + var settings = new RefitSettings { HttpMessageHandlerFactory = () => mockHttp }; + + mockHttp + .Expect(HttpMethod.Get, "http://foo/resource") + .Respond(HttpStatusCode.OK); + + var fixture = RestService.For("http://foo", settings); + + fixture.DoVoid(); // should not throw + + mockHttp.VerifyNoOutstandingExpectation(); + } } diff --git a/Refit/RequestBuilderImplementation.cs b/Refit/RequestBuilderImplementation.cs index f2b3f9df4..85a1870ef 100644 --- a/Refit/RequestBuilderImplementation.cs +++ b/Refit/RequestBuilderImplementation.cs @@ -281,57 +281,191 @@ RestMethodInfoInternal CloseGenericMethodIfNeeded( RestMethodInfoInternal restMethod ) { - var factory = BuildRequestFactoryForMethod( - restMethod, - string.Empty, - paramsContainsCancellationToken: false + // void return: mirror BuildVoidTaskFuncForMethod – run ExceptionFactory, return null + if (restMethod.ReturnResultType == typeof(void)) + { + return (client, paramList) => + { + if (client.BaseAddress == null) + throw new InvalidOperationException( + "BaseAddress must be set on the HttpClient instance" + ); + + var factory = BuildRequestFactoryForMethod( + restMethod, + client.BaseAddress.AbsolutePath, + paramsContainsCancellationToken: false + ); + using var rq = factory(paramList); + using var resp = client + .SendAsync(rq, HttpCompletionOption.ResponseHeadersRead) + .GetAwaiter() + .GetResult(); + var e = settings.ExceptionFactory(resp).GetAwaiter().GetResult(); + if (e != null) + throw e; + return null; + }; + } + + // Non-void: dispatch to the generic implementation that replicates the full + // BuildCancellableTaskFuncForMethod pipeline (ExceptionFactory, IsApiResponse, + // ShouldDisposeResponse, DeserializationExceptionFactory). + var syncFuncMi = typeof(RequestBuilderImplementation).GetMethod( + nameof(BuildGeneratedSyncFuncForMethodGeneric), + BindingFlags.NonPublic | BindingFlags.Instance ); + return (Func) + syncFuncMi! + .MakeGenericMethod( + restMethod.ReturnResultType, + restMethod.DeserializedResultType + ) + .Invoke(this, [restMethod])!; + } + Func BuildGeneratedSyncFuncForMethodGeneric( + RestMethodInfoInternal restMethod + ) + { return (client, paramList) => { - using var rq = factory(paramList); - var resp = client.SendAsync(rq).GetAwaiter().GetResult(); + if (client.BaseAddress == null) + throw new InvalidOperationException( + "BaseAddress must be set on the HttpClient instance" + ); + var factory = BuildRequestFactoryForMethod( + restMethod, + client.BaseAddress.AbsolutePath, + paramsContainsCancellationToken: false + ); + var rq = factory(paramList); + HttpResponseMessage? resp = null; + HttpContent? content = null; + var disposeResponse = true; try { - if (restMethod.ReturnResultType == typeof(void)) + if (IsBodyBuffered(restMethod, rq)) + { + rq.Content!.LoadIntoBufferAsync().GetAwaiter().GetResult(); + } + resp = client + .SendAsync(rq, HttpCompletionOption.ResponseHeadersRead) + .GetAwaiter() + .GetResult(); + content = resp.Content ?? new StringContent(string.Empty); + Exception? e = null; + disposeResponse = restMethod.ShouldDisposeResponse; + + if (typeof(T) != typeof(HttpResponseMessage)) { - return null; + e = settings.ExceptionFactory(resp).GetAwaiter().GetResult(); } - return DeserializeSyncResponse(resp, restMethod.DeserializedResultType); + if (restMethod.IsApiResponse) + { + var body = default(TBody); + try + { + body = + e == null + ? DeserializeContentSync(resp, content) + : default; + } + catch (Exception ex) + { + if (settings.DeserializationExceptionFactory != null) + e = settings + .DeserializationExceptionFactory(resp, ex) + .GetAwaiter() + .GetResult(); + else + { + e = ApiException + .Create( + "An error occured deserializing the response.", + resp.RequestMessage!, + resp.RequestMessage!.Method, + resp, + settings, + ex + ) + .GetAwaiter() + .GetResult(); + } + } + + return ApiResponse.Create(resp, body, settings, e as ApiException); + } + else if (e != null) + { + disposeResponse = false; // caller must dispose + throw e; + } + else + { + try + { + return DeserializeContentSync(resp, content); + } + catch (Exception ex) + { + if (settings.DeserializationExceptionFactory != null) + { + var customEx = settings + .DeserializationExceptionFactory(resp, ex) + .GetAwaiter() + .GetResult(); + if (customEx != null) + throw customEx; + return default; + } + else + { + throw ApiException + .Create( + "An error occured deserializing the response.", + resp.RequestMessage!, + resp.RequestMessage!.Method, + resp, + settings, + ex + ) + .GetAwaiter() + .GetResult(); + } + } + } } finally { - resp.Dispose(); + rq.Dispose(); + if (disposeResponse) + { + resp?.Dispose(); + content?.Dispose(); + } } }; } - object? DeserializeSyncResponse(HttpResponseMessage response, Type resultType) - { - if (resultType == typeof(HttpContent)) - { - return response.Content; - } - - var deserializeMethod = typeof(RequestBuilderImplementation).GetMethod( - nameof(DeserializeSyncResponseGeneric), - BindingFlags.NonPublic | BindingFlags.Instance - )!; - - return deserializeMethod.MakeGenericMethod(resultType).Invoke(this, [response]); - } - - T? DeserializeSyncResponseGeneric(HttpResponseMessage response) + T? DeserializeContentSync(HttpResponseMessage resp, HttpContent content) { + if (typeof(T) == typeof(HttpResponseMessage)) + return (T)(object)resp; + if (typeof(T) == typeof(HttpContent)) + return (T)(object)content; + if (typeof(T) == typeof(Stream)) + return (T)(object)content.ReadAsStreamAsync().GetAwaiter().GetResult(); if (typeof(T) == typeof(string)) { - return (T?)(object?)response.Content.ReadAsStringAsync().GetAwaiter().GetResult(); + using var stream = content.ReadAsStreamAsync().GetAwaiter().GetResult(); + using var reader = new StreamReader(stream); + return (T)(object)reader.ReadToEnd(); } - return settings.ContentSerializer - .FromHttpContentAsync(response.Content) + .FromHttpContentAsync(content) .GetAwaiter() .GetResult(); } diff --git a/Refit/RestMethodInfo.cs b/Refit/RestMethodInfo.cs index 8fcda9432..950394bb2 100644 --- a/Refit/RestMethodInfo.cs +++ b/Refit/RestMethodInfo.cs @@ -688,9 +688,25 @@ void DetermineReturnTypeInfo(MethodInfo methodInfo) ReturnType = methodInfo.ReturnType; ReturnResultType = methodInfo.ReturnType; - DeserializedResultType = methodInfo.ReturnType == typeof(IApiResponse) - ? typeof(HttpContent) - : methodInfo.ReturnType; + + if ( + ReturnResultType.IsGenericType + && ( + ReturnResultType.GetGenericTypeDefinition() == typeof(ApiResponse<>) + || ReturnResultType.GetGenericTypeDefinition() == typeof(IApiResponse<>) + ) + ) + { + DeserializedResultType = ReturnResultType.GetGenericArguments()[0]; + } + else if (ReturnResultType == typeof(IApiResponse)) + { + DeserializedResultType = typeof(HttpContent); + } + else + { + DeserializedResultType = ReturnResultType; + } } } From 430e4a1a87cc82c23420a1ff66e991b025a5aead Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 25 Mar 2026 16:34:47 +0000 Subject: [PATCH 3/3] Fix spelling: 'occured' -> 'occurred' in new sync error messages Co-authored-by: ChrisPulman <4910015+ChrisPulman@users.noreply.github.com> Agent-Logs-Url: https://github.com/reactiveui/refit/sessions/7e184030-9d18-44ec-aff4-305b2c7e973d --- Refit/RequestBuilderImplementation.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Refit/RequestBuilderImplementation.cs b/Refit/RequestBuilderImplementation.cs index 85a1870ef..fea2179bd 100644 --- a/Refit/RequestBuilderImplementation.cs +++ b/Refit/RequestBuilderImplementation.cs @@ -384,7 +384,7 @@ RestMethodInfoInternal restMethod { e = ApiException .Create( - "An error occured deserializing the response.", + "An error occurred deserializing the response.", resp.RequestMessage!, resp.RequestMessage!.Method, resp, @@ -425,7 +425,7 @@ RestMethodInfoInternal restMethod { throw ApiException .Create( - "An error occured deserializing the response.", + "An error occurred deserializing the response.", resp.RequestMessage!, resp.RequestMessage!.Method, resp,