Fix sync method pipeline to match async behavior (ExceptionFactory, IApiResponse, disposal) (#2070)

* Initial plan

* 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<T,TBody> 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<T> handling all content types:
  HttpResponseMessage, HttpContent, Stream, string, and deserialised T.
- Fix RestMethodInfo sync branch to properly decompose IApiResponse<T> /
  ApiResponse<T> return types, setting DeserializedResultType to the
  inner type just like the async Task<IApiResponse<T>> 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<T>, 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

* 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

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: ChrisPulman <4910015+ChrisPulman@users.noreply.github.com>

Author: Copilot

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}
             """
         );
 

InterfaceStubGenerator.Shared/Models/MethodModel.cs

@@ -17,5 +17,6 @@ internal enum ReturnTypeInfo : byte
 {
     Return,
     AsyncVoid,
-    AsyncResult
+    AsyncResult,
+    SyncVoid
 }

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,
         };
 

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<string> 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<ISyncPipelineApi>("http://foo", settings);
+
+        var ex = Assert.Throws<ApiException>(() => 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<ISyncPipelineApi>("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<ISyncPipelineApi>("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<ISyncPipelineApi>("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<ISyncPipelineApi>("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<ISyncPipelineApi>("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<ISyncPipelineApi>("http://foo", settings);
+
+        var ex = Assert.Throws<ApiException>(() => 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<ISyncPipelineApi>("http://foo", settings);
+
+        fixture.DoVoid(); // should not throw
+
+        mockHttp.VerifyNoOutstandingExpectation();
+    }
 }