From 1aed1f8e8e93d7d598c07da6a139491d9b81bcac Mon Sep 17 00:00:00 2001 From: Adolfo Marinucci Date: Thu, 15 Oct 2026 12:40:35 +0200 Subject: [PATCH 1/3] Carry a machine-readable reason on every sync API failure The sync API keeps the status codes clients have always received (404 for an unknown endpoint or an expired upload/download session, 503 for an offline agent, 422 for an unsupported store configuration) and adds the X-CoreSync-Error / X-CoreSync-Trace headers plus a SyncErrorDetails body, so a current CoreSync.Http.Client can tell the user what happened instead of "check your connection". Unexpected exceptions still propagate to the host (exception handler, Sentry) and only get the headers attached to the host's response. AgentProxySyncProvider now tells an agent that answered with an error (HubException) apart from an agent that is offline or did not answer: the former is a failure of the data store behind the agent and propagates like any other provider failure instead of being reported as 503 agent offline. Bumps the CoreSync submodule to the commit introducing the error classification. --- CoreSync | 2 +- src/Services/AgentOfflineException.cs | 4 +- src/Services/AgentOperationFailedException.cs | 26 ++++ src/Services/Controllers/SyncController.cs | 58 ++++++-- .../SyncConfigurationExceptionFilter.cs | 6 + .../Filters/SyncEndpointAuthFilter.cs | 12 +- .../Filters/SyncErrorResponseFilter.cs | 48 ++++++ src/Services/Filters/SyncErrorResponses.cs | 99 +++++++++++++ .../Implementation/AgentProxySyncProvider.cs | 32 ++++ src/Services/ServiceCollectionExtensions.cs | 3 + .../SyncControllerTests.cs | 138 +++++++++++++++++- 11 files changed, 401 insertions(+), 27 deletions(-) create mode 100644 src/Services/AgentOperationFailedException.cs create mode 100644 src/Services/Filters/SyncErrorResponseFilter.cs create mode 100644 src/Services/Filters/SyncErrorResponses.cs diff --git a/CoreSync b/CoreSync index 9902bd4..29a0438 160000 --- a/CoreSync +++ b/CoreSync @@ -1 +1 @@ -Subproject commit 9902bd4f860bcf2eb0462d384e475aedeaa64d1e +Subproject commit 29a0438e85662feb51dd05b81df377c06bf2dc56 diff --git a/src/Services/AgentOfflineException.cs b/src/Services/AgentOfflineException.cs index 6cc0a00..6dc5498 100644 --- a/src/Services/AgentOfflineException.cs +++ b/src/Services/AgentOfflineException.cs @@ -2,7 +2,9 @@ namespace CoreSyncServer.Services; /// /// Thrown by AgentProxySyncProvider when no agent connection is registered for the target DataStore, -/// or the RPC call to the connected agent fails. SyncController maps this to HTTP 503. +/// or the RPC call to the connected agent does not complete (timeout, dropped connection). +/// SyncController maps this to HTTP 503 with the agent-unavailable code. +/// An agent that answers with an error raises instead. /// public class AgentOfflineException : Exception { diff --git a/src/Services/AgentOperationFailedException.cs b/src/Services/AgentOperationFailedException.cs new file mode 100644 index 0000000..0aa1982 --- /dev/null +++ b/src/Services/AgentOperationFailedException.cs @@ -0,0 +1,26 @@ +namespace CoreSyncServer.Services; + +/// +/// Thrown by AgentProxySyncProvider when the agent was reached, executed the requested +/// operation and reported that it failed - typically its sync provider rejecting the changes against +/// the on-premise database. +/// +/// +/// Kept apart from on purpose: that one means "nobody answered", +/// which is transient and answered with 503, while this one is a failure of the data store behind the +/// agent and is treated like any other provider failure - it propagates (500, reported to the host's +/// error reporting) and reaches the client tagged with an error code instead of as "agent offline". +/// +public class AgentOperationFailedException : Exception +{ + public int DataStoreId { get; } + + public string Operation { get; } + + public AgentOperationFailedException(int dataStoreId, string operation, Exception inner) + : base($"Agent for DataStore {dataStoreId} reported a failure executing '{operation}': {inner.Message}", inner) + { + DataStoreId = dataStoreId; + Operation = operation; + } +} diff --git a/src/Services/Controllers/SyncController.cs b/src/Services/Controllers/SyncController.cs index b3ce896..372289d 100644 --- a/src/Services/Controllers/SyncController.cs +++ b/src/Services/Controllers/SyncController.cs @@ -16,11 +16,21 @@ namespace CoreSyncServer.Controllers; +/// +/// The CoreSync HTTP wire protocol, served per published endpoint. +/// +/// +/// Failures the controller recognises are answered through : the same +/// status code as before, plus the X-CoreSync-Error / X-CoreSync-Trace headers and a +/// body a current client turns into a typed failure. Anything else +/// propagates to the host (exception handler, Sentry) as it always did, tagged with the same headers +/// by . +/// [ApiController] [AllowAnonymous] [Route("api/sync/{endpointId:guid}")] [ServiceFilter(typeof(SyncEndpointAuthFilter))] -[ServiceFilter(typeof(SyncConfigurationExceptionFilter))] +[ServiceFilter(typeof(SyncErrorResponseFilter))] public class SyncController( ApplicationDbContext context, ISyncProviderFactory syncProviderFactory, @@ -132,7 +142,7 @@ public async Task> GetStoreId(Guid endpointId, Cancellation } catch (KeyNotFoundException) { - return NotFound(); + return EndpointNotFound(endpointId); } catch (AgentOfflineException ex) { @@ -151,7 +161,7 @@ public async Task> GetSyncVersion(Guid endpointId, Can } catch (KeyNotFoundException) { - return NotFound(); + return EndpointNotFound(endpointId); } catch (AgentOfflineException ex) { @@ -159,10 +169,28 @@ public async Task> GetSyncVersion(Guid endpointId, Can } } + /// + /// 404 with the endpoint-not-found code: the status an unknown endpoint always produced, + /// now distinguishable from an expired session, which is also a 404. + /// + private ObjectResult EndpointNotFound(Guid endpointId) + => SyncErrorResponses.EndpointNotFound(HttpContext, endpointId); + + /// + /// 404 with the session-expired code: the status an unknown upload/download session always + /// produced. The client is told to start a new synchronization rather than retry the request. + /// + private ObjectResult SessionExpired(Guid sessionId) + => SyncErrorResponses.SessionExpired(HttpContext, sessionId); + + /// + /// 503 with the agent-unavailable code: the agent owning the data store is not connected or + /// did not answer in time. Transient from the client's point of view. + /// private ObjectResult AgentOffline(AgentOfflineException ex) { logger.LogWarning(ex, "Agent offline for DataStore {DataStoreId}", ex.DataStoreId); - return StatusCode(StatusCodes.Status503ServiceUnavailable, ex.Message); + return SyncErrorResponses.AgentUnavailable(HttpContext, ex.Message); } [HttpGet("changes-bulk/{storeId:guid}")] @@ -215,7 +243,7 @@ public async Task> GetBulkChangeSet(Guid endpoin } catch (KeyNotFoundException) { - return NotFound(); + return EndpointNotFound(endpointId); } catch (AgentOfflineException ex) { @@ -242,7 +270,7 @@ public ActionResult> GetBulkChangeSetItem(Guid sessionId return Ok(bufferList); } - return NotFound(); + return SessionExpired(sessionId); } [HttpGet("changes-bulk-item-binary/{sessionId:guid}/{skip:int}/{take:int}")] @@ -265,7 +293,7 @@ public ActionResult GetBulkChangeSetItemBinary(Guid sessionId, int skip, int tak return File(bytes, "application/x-msgpack"); } - return NotFound(); + return SessionExpired(sessionId); } [HttpPost("changes-bulk-begin")] @@ -288,7 +316,7 @@ public async Task BeginApplyBulkChanges(Guid endpointId, [FromBody } catch (KeyNotFoundException) { - return NotFound(); + return EndpointNotFound(endpointId); } } @@ -301,7 +329,7 @@ public ActionResult ApplyBulkChangesItem([FromBody] BulkChangeSetUploadItem bulk return Ok(); } - return NotFound(); + return SessionExpired(bulkUploadItem.SessionId); } [HttpPost("changes-bulk-item-binary")] @@ -319,7 +347,7 @@ public async Task ApplyBulkChangesItemBinary() return Ok(); } - return NotFound(); + return SessionExpired(bulkUploadItem.SessionId); } [HttpPost("changes-bulk-complete/{sessionId:guid}")] @@ -360,7 +388,7 @@ public async Task> CompleteApplyBulkChanges(Guid endpoi // Cleanup on CancellationToken.None — see GetBulkChangeSet for the rationale. await sessionLogger.FlushAsync(CancellationToken.None); await syncSessionService.ErrorAsync(cached.SyncSessionId, $"Endpoint '{endpointId}' not found.", CancellationToken.None); - return NotFound(); + return EndpointNotFound(endpointId); } catch (AgentOfflineException ex) { @@ -376,7 +404,7 @@ public async Task> CompleteApplyBulkChanges(Guid endpoi } } - return NotFound(); + return SessionExpired(sessionId); } [HttpPost("changes-bulk-complete-binary/{sessionId:guid}")] @@ -406,7 +434,7 @@ public async Task> CompleteApplyBulkChangesBinary(Guid // Cleanup on CancellationToken.None — see GetBulkChangeSet for the rationale. await sessionLogger.FlushAsync(CancellationToken.None); await syncSessionService.ErrorAsync(cached.SyncSessionId, $"Endpoint '{endpointId}' not found.", CancellationToken.None); - return NotFound(); + return EndpointNotFound(endpointId); } catch (AgentOfflineException ex) { @@ -422,7 +450,7 @@ public async Task> CompleteApplyBulkChangesBinary(Guid } } - return NotFound(); + return SessionExpired(sessionId); } [HttpPost("save-version/{storeId:guid}/{version:long}")] @@ -439,7 +467,7 @@ public async Task SaveVersionForStore(Guid endpointId, Guid storeI } catch (KeyNotFoundException) { - return NotFound(); + return EndpointNotFound(endpointId); } catch (AgentOfflineException ex) { diff --git a/src/Services/Filters/SyncConfigurationExceptionFilter.cs b/src/Services/Filters/SyncConfigurationExceptionFilter.cs index ff608d3..e037c11 100644 --- a/src/Services/Filters/SyncConfigurationExceptionFilter.cs +++ b/src/Services/Filters/SyncConfigurationExceptionFilter.cs @@ -11,6 +11,12 @@ namespace CoreSyncServer.Filters; /// 422 Unprocessable Entity response carrying the provider's message, instead of letting them /// surface to the sync client as an opaque HTTP 500. /// +/// +/// Superseded by , which answers the same 422 with the +/// configuration-error code and a body and also +/// tags every other failure. Kept registered so hosts that reference it keep compiling. +/// +[Obsolete("Use SyncErrorResponseFilter, which covers this case and every other sync failure.")] public class SyncConfigurationExceptionFilter(ILogger logger) : IExceptionFilter { public void OnException(ExceptionContext context) diff --git a/src/Services/Filters/SyncEndpointAuthFilter.cs b/src/Services/Filters/SyncEndpointAuthFilter.cs index 1f96947..845ecda 100644 --- a/src/Services/Filters/SyncEndpointAuthFilter.cs +++ b/src/Services/Filters/SyncEndpointAuthFilter.cs @@ -29,15 +29,11 @@ public async Task OnActionExecutionAsync(ActionExecutingContext filterContext, A .Include(e => e.Authentication) .FirstOrDefaultAsync(e => e.Id == endpointId, filterContext.HttpContext.RequestAborted); - if (endpoint is null) + // Same 404 as before, now carrying the endpoint-not-found code and a body: without a body the + // host's status code pages would replace it with the HTML not-found page. + if (endpoint is null || !endpoint.IsPublished) { - filterContext.Result = new NotFoundResult(); - return; - } - - if (!endpoint.IsPublished) - { - filterContext.Result = new NotFoundResult(); + filterContext.Result = SyncErrorResponses.EndpointNotFound(filterContext.HttpContext, endpointId); return; } diff --git a/src/Services/Filters/SyncErrorResponseFilter.cs b/src/Services/Filters/SyncErrorResponseFilter.cs new file mode 100644 index 0000000..6463256 --- /dev/null +++ b/src/Services/Filters/SyncErrorResponseFilter.cs @@ -0,0 +1,48 @@ +using Microsoft.AspNetCore.Mvc.Filters; +using Microsoft.Extensions.Logging; + +namespace CoreSyncServer.Filters; + +/// +/// Makes every failure escaping SyncController classifiable by the client. +/// +/// +/// +/// +/// - a store configured in a way the provider cannot synchronize +/// (composite primary key, reserved column name...) - becomes 422 Unprocessable Entity with the +/// configuration-error code and a body carrying the +/// provider's message, the status this API has always returned for it. +/// +/// +/// Everything else is left unhandled, so the host's exception handler, logging and error reporting +/// (Sentry) see it exactly as before; only the error code and trace headers are attached to the host's +/// response (see ). +/// +/// +/// +public class SyncErrorResponseFilter(ILogger logger) : IExceptionFilter +{ + public void OnException(ExceptionContext context) + { + if (context.ExceptionHandled) + return; + + if (context.Exception is NotSupportedException notSupported) + { + logger.LogWarning(notSupported, "Unsupported sync configuration: {Message}", notSupported.Message); + + context.Result = SyncErrorResponses.ConfigurationError(context.HttpContext, notSupported.Message); + context.ExceptionHandled = true; + return; + } + + if (context.Exception is OperationCanceledException && context.HttpContext.RequestAborted.IsCancellationRequested) + { + // The client went away; there is no response to tag. + return; + } + + SyncErrorResponses.TagUnhandled(context.HttpContext, context.Exception); + } +} diff --git a/src/Services/Filters/SyncErrorResponses.cs b/src/Services/Filters/SyncErrorResponses.cs new file mode 100644 index 0000000..e58fc12 --- /dev/null +++ b/src/Services/Filters/SyncErrorResponses.cs @@ -0,0 +1,99 @@ +using CoreSync; +using CoreSync.Http; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Mvc; + +namespace CoreSyncServer.Filters; + +/// +/// Builds the responses the sync API returns for a failure it recognises: the status code the client +/// always received for that case, plus the and +/// headers and a body that a +/// current CoreSync.Http.Client turns into a typed failure the app can explain to the user. +/// +/// +/// Status codes are deliberately unchanged from what the controller returned before the signal +/// existed (404 for an unknown endpoint or session, 503 for an offline agent, 422 for an unsupported +/// configuration): an older client only looks at the status, and must keep seeing what it saw. The +/// body also keeps the host's UseStatusCodePagesWithReExecute from replacing a 404 with the +/// HTML not-found page, which has no content type a sync client could use. +/// +public static class SyncErrorResponses +{ + public static ObjectResult EndpointNotFound(HttpContext httpContext, Guid endpointId) + => Create(httpContext, StatusCodes.Status404NotFound, SyncHttpErrorCodes.EndpointNotFound, + $"Synchronization endpoint '{endpointId}' does not exist or is not published."); + + public static ObjectResult SessionExpired(HttpContext httpContext, Guid sessionId) + => Create(httpContext, StatusCodes.Status404NotFound, SyncHttpErrorCodes.SessionExpired, + $"Synchronization session '{sessionId}' is no longer available on the server. Start a new synchronization."); + + public static ObjectResult AgentUnavailable(HttpContext httpContext, string message) + => Create(httpContext, StatusCodes.Status503ServiceUnavailable, SyncHttpErrorCodes.AgentUnavailable, message); + + public static ObjectResult ConfigurationError(HttpContext httpContext, string message) + => Create(httpContext, StatusCodes.Status422UnprocessableEntity, SyncHttpErrorCodes.ConfigurationError, message); + + public static ObjectResult Create(HttpContext httpContext, int statusCode, string code, string message, string? tableName = null) + { + var details = new SyncErrorDetails + { + Code = code, + Message = message, + TableName = tableName, + TraceId = httpContext.TraceIdentifier + }; + + httpContext.Response.Headers[SyncHttpHeaders.ErrorCode] = code; + httpContext.Response.Headers[SyncHttpHeaders.TraceId] = details.TraceId; + + return new ObjectResult(details) { StatusCode = statusCode }; + } + + /// + /// Arranges for the error code and trace headers to be on the response the host produces for an + /// exception this API does not handle, without handling it: the exception keeps propagating to + /// UseExceptionHandler, the Sentry middleware and the request logger exactly as before. + /// + /// + /// The exception handler middleware clears every header already set when it takes over, so the + /// headers are added from an callback, which it does not + /// clear and which runs when its own error response starts. + /// + public static void TagUnhandled(HttpContext httpContext, Exception exception) + { + var code = Classify(exception); + var traceId = httpContext.TraceIdentifier; + var response = httpContext.Response; + + response.OnStarting(() => + { + if (!response.Headers.ContainsKey(SyncHttpHeaders.ErrorCode)) + { + response.Headers[SyncHttpHeaders.ErrorCode] = code; + response.Headers[SyncHttpHeaders.TraceId] = traceId; + } + + return Task.CompletedTask; + }); + } + + /// + /// Picks the error code for an exception the controller did not expect: a provider's + /// means the store refused a change; anything else is an + /// unclassified server failure, which still tells the client the device is online. + /// + public static string Classify(Exception exception) + { + for (var current = exception; current != null; current = current.InnerException) + { + if (current is SynchronizationException) + return SyncHttpErrorCodes.DataError; + + if (current is NotSupportedException) + return SyncHttpErrorCodes.ConfigurationError; + } + + return SyncHttpErrorCodes.ServerError; + } +} diff --git a/src/Services/Implementation/AgentProxySyncProvider.cs b/src/Services/Implementation/AgentProxySyncProvider.cs index 722a79f..7f96738 100644 --- a/src/Services/Implementation/AgentProxySyncProvider.cs +++ b/src/Services/Implementation/AgentProxySyncProvider.cs @@ -12,6 +12,13 @@ namespace CoreSyncServer.Services.Implementation; /// Analogous to CoreSync.Http.Client's SyncProviderHttpClient, but over the /// persistent agent connection rather than HTTP. Provisioning and retention APIs are out of /// scope for the initial iteration and throw . +/// +/// Two failure shapes are told apart: when no agent is connected +/// or the RPC did not complete (timeout, dropped connection), and +/// when the agent answered with an error - SignalR +/// surfaces the agent handler's exception as a - which is a failure of +/// the data store behind it, not of its availability. +/// /// internal sealed class AgentProxySyncProvider : ISyncProvider { @@ -52,6 +59,11 @@ public async Task GetStoreIdAsync(CancellationToken cancellationToken = de { return await client.SyncGetStoreId(_tables).WaitAsync(RpcTimeout, cancellationToken); } + catch (HubException ex) + { + // The agent answered: its provider failed. Not an availability problem. + throw new AgentOperationFailedException(_dataStoreId, "SyncGetStoreId", ex); + } catch (Exception ex) when (ex is not AgentOfflineException and not OperationCanceledException) { throw new AgentOfflineException(_dataStoreId, $"Agent RPC 'SyncGetStoreId' failed for DataStore {_dataStoreId}.", ex); @@ -65,6 +77,11 @@ public async Task GetSyncVersionAsync(CancellationToken cancellatio { return await client.SyncGetSyncVersion(_tables).WaitAsync(RpcTimeout, cancellationToken); } + catch (HubException ex) + { + // The agent answered: its provider failed. Not an availability problem. + throw new AgentOperationFailedException(_dataStoreId, "SyncGetSyncVersion", ex); + } catch (Exception ex) when (ex is not AgentOfflineException and not OperationCanceledException) { throw new AgentOfflineException(_dataStoreId, $"Agent RPC 'SyncGetSyncVersion' failed for DataStore {_dataStoreId}.", ex); @@ -89,6 +106,11 @@ public async Task GetChangesAsync( SyncPayloadConverter.NormalizeChangeSet(changeSet); return changeSet; } + catch (HubException ex) + { + // The agent answered: its provider failed. Not an availability problem. + throw new AgentOperationFailedException(_dataStoreId, "SyncGetChanges", ex); + } catch (Exception ex) when (ex is not AgentOfflineException and not OperationCanceledException) { throw new AgentOfflineException(_dataStoreId, $"Agent RPC 'SyncGetChanges' failed for DataStore {_dataStoreId}.", ex); @@ -113,6 +135,11 @@ public async Task ApplyChangesAsync( return await client.SyncApplyChanges(changeSet, updateResolution, deleteResolution, _tables) .WaitAsync(RpcTimeout, cancellationToken); } + catch (HubException ex) + { + // The agent answered: its provider failed. Not an availability problem. + throw new AgentOperationFailedException(_dataStoreId, "SyncApplyChanges", ex); + } catch (Exception ex) when (ex is not AgentOfflineException and not OperationCanceledException) { throw new AgentOfflineException(_dataStoreId, $"Agent RPC 'SyncApplyChanges' failed for DataStore {_dataStoreId}.", ex); @@ -126,6 +153,11 @@ public async Task SaveVersionForStoreAsync(Guid otherStoreId, long version, Canc { await client.SyncSaveVersionForStore(otherStoreId, version, _tables).WaitAsync(RpcTimeout, cancellationToken); } + catch (HubException ex) + { + // The agent answered: its provider failed. Not an availability problem. + throw new AgentOperationFailedException(_dataStoreId, "SyncSaveVersionForStore", ex); + } catch (Exception ex) when (ex is not AgentOfflineException and not OperationCanceledException) { throw new AgentOfflineException(_dataStoreId, $"Agent RPC 'SyncSaveVersionForStore' failed for DataStore {_dataStoreId}.", ex); diff --git a/src/Services/ServiceCollectionExtensions.cs b/src/Services/ServiceCollectionExtensions.cs index 7995ace..4b9c1a9 100644 --- a/src/Services/ServiceCollectionExtensions.cs +++ b/src/Services/ServiceCollectionExtensions.cs @@ -68,7 +68,10 @@ public static IServiceCollection AddCoreSyncServerServices( services.AddScoped(); services.AddScoped(); +#pragma warning disable CS0618 // kept registered for hosts that still reference it services.AddScoped(); +#pragma warning restore CS0618 + services.AddScoped(); services.AddScoped(); services.AddSingleton(); diff --git a/tests/CoreSyncServer.Tests/SyncControllerTests.cs b/tests/CoreSyncServer.Tests/SyncControllerTests.cs index 04d5e15..cfbc820 100644 --- a/tests/CoreSyncServer.Tests/SyncControllerTests.cs +++ b/tests/CoreSyncServer.Tests/SyncControllerTests.cs @@ -1,4 +1,5 @@ using CoreSync; +using CoreSync.Http; using CoreSync.Http.Client; using CoreSync.Sqlite; using CoreSyncServer.Data; @@ -7,6 +8,8 @@ using Microsoft.Data.Sqlite; using Microsoft.EntityFrameworkCore; using Microsoft.Extensions.DependencyInjection; +using System.Net; +using System.Net.Http.Json; namespace CoreSyncServer.Tests; @@ -219,14 +222,145 @@ public async Task Sync_InvalidEndpoint_ReturnsNotFound() response.StatusCode.Should().Be(System.Net.HttpStatusCode.NotFound); } - private ISyncProviderHttpClient CreateRemoteSyncClient() + // The sync API answers the failures it recognises with the status code it always used plus the + // X-CoreSync-Error / X-CoreSync-Trace headers and a SyncErrorDetails body, so a current client can + // tell the user what happened while an older one keeps seeing the status it expects. + + [Fact] + public async Task Sync_InvalidEndpoint_CarriesEndpointNotFoundSignal() + { + var httpClient = _factory.CreateClient(); + + var response = await httpClient.GetAsync($"api/sync/{Guid.NewGuid()}/store-id"); + + response.StatusCode.Should().Be(HttpStatusCode.NotFound); + response.Headers.GetValues(SyncHttpHeaders.ErrorCode).Should().ContainSingle().Which.Should().Be(SyncHttpErrorCodes.EndpointNotFound); + response.Headers.GetValues(SyncHttpHeaders.TraceId).Should().ContainSingle().Which.Should().NotBeNullOrWhiteSpace(); + + // A JSON body, not the host's HTML not-found page the status code pages would otherwise serve + response.Content.Headers.ContentType!.MediaType.Should().Be("application/json"); + var details = await response.Content.ReadFromJsonAsync(); + details!.Code.Should().Be(SyncHttpErrorCodes.EndpointNotFound); + details.TraceId.Should().Be(response.Headers.GetValues(SyncHttpHeaders.TraceId).Single()); + } + + [Fact] + public async Task Sync_UnpublishedEndpoint_CarriesEndpointNotFoundSignal() + { + Guid unpublishedId; + using (var scope = _factory.Services.CreateScope()) + { + var db = scope.ServiceProvider.GetRequiredService(); + var config = await db.DataStoreConfigurations.FirstAsync(); + var endpoint = new Endpoint { Id = Guid.NewGuid(), Name = "Draft", DataStoreConfigurationId = config.Id, IsPublished = false }; + db.Endpoints.Add(endpoint); + await db.SaveChangesAsync(); + unpublishedId = endpoint.Id; + } + + var response = await _factory.CreateClient().GetAsync($"api/sync/{unpublishedId}/store-id"); + + response.StatusCode.Should().Be(HttpStatusCode.NotFound); + response.Headers.GetValues(SyncHttpHeaders.ErrorCode).Should().ContainSingle(SyncHttpErrorCodes.EndpointNotFound); + } + + [Fact] + public async Task Sync_UnknownDownloadSession_CarriesSessionExpiredSignal() + { + var response = await _factory.CreateClient().GetAsync($"api/sync/{_endpointId}/changes-bulk-item/{Guid.NewGuid()}/0/50"); + + // Still a 404 - that is what an older client has always received here - but no longer + // indistinguishable from an unknown endpoint + response.StatusCode.Should().Be(HttpStatusCode.NotFound); + response.Headers.GetValues(SyncHttpHeaders.ErrorCode).Should().ContainSingle(SyncHttpErrorCodes.SessionExpired); + + var details = await response.Content.ReadFromJsonAsync(); + details!.Code.Should().Be(SyncHttpErrorCodes.SessionExpired); + details.Message.Should().NotBeNullOrWhiteSpace(); + } + + [Fact] + public async Task Sync_UnknownUploadSession_CarriesSessionExpiredSignal() + { + var httpClient = _factory.CreateClient(); + + var complete = await httpClient.PostAsync($"api/sync/{_endpointId}/changes-bulk-complete/{Guid.NewGuid()}", null); + complete.StatusCode.Should().Be(HttpStatusCode.NotFound); + complete.Headers.GetValues(SyncHttpHeaders.ErrorCode).Should().ContainSingle(SyncHttpErrorCodes.SessionExpired); + + var item = await httpClient.PostAsJsonAsync($"api/sync/{_endpointId}/changes-bulk-item", new BulkChangeSetUploadItem { SessionId = Guid.NewGuid(), Items = [] }); + item.StatusCode.Should().Be(HttpStatusCode.NotFound); + item.Headers.GetValues(SyncHttpHeaders.ErrorCode).Should().ContainSingle(SyncHttpErrorCodes.SessionExpired); + } + + [Fact] + public async Task Sync_UnknownEndpoint_IsReportedAsConfigurationErrorByTheAgent() + { + var remoteSyncClient = CreateRemoteSyncClient(Guid.NewGuid()); + var localProvider = CreateSqliteSyncProvider(_localDbPath, ProviderMode.Local); + + var syncAgent = new SyncAgent(localProvider, remoteSyncClient); + var act = () => syncAgent.SynchronizeAsync(); + + var ex = (await act.Should().ThrowAsync()).Which; + + // What the app can now tell the user, and what it could before (an HttpRequestException) + ex.Kind.Should().Be(SyncFailureKind.ConfigurationError); + ex.Stage.Should().Be(SynchronizationStage.RemoteStoreId); + ex.ErrorCode.Should().Be(SyncHttpErrorCodes.EndpointNotFound); + ex.TraceId.Should().NotBeNullOrWhiteSpace(); + ex.ServerMessage.Should().Contain("not published"); + ex.InnerException.Should().BeAssignableTo(); + } + + [Fact] + public async Task Sync_ProviderFailure_TagsTheHostErrorResponse() + { + // An endpoint whose configuration names a table the database does not have: the provider + // throws, the exception propagates to the host's exception handler as before (Sentry still + // sees it), and the response it produces carries the error code and trace headers. + Guid brokenEndpointId; + using (var scope = _factory.Services.CreateScope()) + { + var db = scope.ServiceProvider.GetRequiredService(); + var dataStore = await db.DataStores.FirstAsync(); + + var config = new DataStoreConfiguration { Name = "Broken Config", DataStoreId = dataStore.Id }; + db.DataStoreConfigurations.Add(config); + await db.SaveChangesAsync(); + + config.TableConfigurations.Add(new DataStoreTableConfiguration + { + Name = "MissingTable", + SyncMode = DataStoreTableConfigurationSyncMode.UploadAndDownload, + Sort = 0, + DataStoreConfigurationId = config.Id + }); + await db.SaveChangesAsync(); + + var endpoint = new Endpoint { Id = Guid.NewGuid(), Name = "Broken Endpoint", DataStoreConfigurationId = config.Id, IsPublished = true }; + db.Endpoints.Add(endpoint); + await db.SaveChangesAsync(); + brokenEndpointId = endpoint.Id; + } + + var response = await _factory.CreateClient().GetAsync($"api/sync/{brokenEndpointId}/changes-bulk/{Guid.NewGuid()}"); + + response.StatusCode.Should().Be(HttpStatusCode.InternalServerError); + response.Headers.GetValues(SyncHttpHeaders.ErrorCode).Should().ContainSingle(SyncHttpErrorCodes.ServerError); + response.Headers.GetValues(SyncHttpHeaders.TraceId).Should().ContainSingle().Which.Should().NotBeNullOrWhiteSpace(); + } + + private ISyncProviderHttpClient CreateRemoteSyncClient() => CreateRemoteSyncClient(_endpointId); + + private ISyncProviderHttpClient CreateRemoteSyncClient(Guid endpointId) { var httpClient = _factory.CreateClient(); var services = new ServiceCollection(); services.AddSingleton(new TestHttpClientFactory(httpClient)); services.AddCoreSyncHttpClient(options => { - options.SyncControllerRoute = $"api/sync/{_endpointId}"; + options.SyncControllerRoute = $"api/sync/{endpointId}"; options.BulkItemSize = 50; }); var sp = services.BuildServiceProvider(); From 6045a2f19218a2bad3ace3cc14f68c5b2f9f8e53 Mon Sep 17 00:00:00 2001 From: Adolfo Marinucci Date: Thu, 15 Oct 2026 15:16:34 +0200 Subject: [PATCH 2/3] Keep agent-reported failures on the 503 clients already handle A server-only deploy must look exactly like today to the 1.0.1 clients in the field, so an agent that answers a sync RPC with an error is again a 503 with the same body and the same warning log as before, instead of propagating as an unhandled 500. AgentOperationFailedException now derives from AgentOfflineException with the legacy message; what tells the two cases apart is only the new agent-error code header (not permanent, so clients keep retrying as they did) next to agent-unavailable for an agent that did not answer. Also guards the OnStarting header registration against a response that has already started and documents that a bare Kestrel 500 carries no header. Bumps the CoreSync submodule. --- CoreSync | 2 +- src/Services/AgentOfflineException.cs | 5 +- src/Services/AgentOperationFailedException.cs | 16 +++--- src/Services/Controllers/SyncController.cs | 9 ++-- src/Services/Filters/SyncErrorResponses.cs | 30 +++++++++-- .../Implementation/AgentProxySyncProvider.cs | 15 +++--- .../AgentFailureResponseTests.cs | 52 +++++++++++++++++++ 7 files changed, 101 insertions(+), 28 deletions(-) create mode 100644 tests/CoreSyncServer.Tests/AgentFailureResponseTests.cs diff --git a/CoreSync b/CoreSync index 29a0438..2cc313d 160000 --- a/CoreSync +++ b/CoreSync @@ -1 +1 @@ -Subproject commit 29a0438e85662feb51dd05b81df377c06bf2dc56 +Subproject commit 2cc313db0d80f8bd35cab8baf249b17a8cf0be19 diff --git a/src/Services/AgentOfflineException.cs b/src/Services/AgentOfflineException.cs index 6dc5498..c163d42 100644 --- a/src/Services/AgentOfflineException.cs +++ b/src/Services/AgentOfflineException.cs @@ -1,10 +1,11 @@ -namespace CoreSyncServer.Services; +namespace CoreSyncServer.Services; /// /// Thrown by AgentProxySyncProvider when no agent connection is registered for the target DataStore, /// or the RPC call to the connected agent does not complete (timeout, dropped connection). /// SyncController maps this to HTTP 503 with the agent-unavailable code. -/// An agent that answers with an error raises instead. +/// An agent that answers with an error raises the derived , +/// mapped to the same 503 with the agent-error code. /// public class AgentOfflineException : Exception { diff --git a/src/Services/AgentOperationFailedException.cs b/src/Services/AgentOperationFailedException.cs index 0aa1982..0b2d851 100644 --- a/src/Services/AgentOperationFailedException.cs +++ b/src/Services/AgentOperationFailedException.cs @@ -6,21 +6,19 @@ namespace CoreSyncServer.Services; /// the on-premise database. /// /// -/// Kept apart from on purpose: that one means "nobody answered", -/// which is transient and answered with 503, while this one is a failure of the data store behind the -/// agent and is treated like any other provider failure - it propagates (500, reported to the host's -/// error reporting) and reaches the client tagged with an error code instead of as "agent offline". +/// Derives from and carries the very message that type carried +/// for this case, so SyncController keeps answering HTTP 503 with the same body and logging +/// it the same way: a server deployed ahead of its clients must look identical to them. What it adds +/// is the distinction itself, which the controller turns into the agent-error code header so +/// that a current client no longer reads an agent-side failure as "agent offline". /// -public class AgentOperationFailedException : Exception +public class AgentOperationFailedException : AgentOfflineException { - public int DataStoreId { get; } - public string Operation { get; } public AgentOperationFailedException(int dataStoreId, string operation, Exception inner) - : base($"Agent for DataStore {dataStoreId} reported a failure executing '{operation}': {inner.Message}", inner) + : base(dataStoreId, $"Agent RPC '{operation}' failed for DataStore {dataStoreId}.", inner) { - DataStoreId = dataStoreId; Operation = operation; } } diff --git a/src/Services/Controllers/SyncController.cs b/src/Services/Controllers/SyncController.cs index 372289d..fe43400 100644 --- a/src/Services/Controllers/SyncController.cs +++ b/src/Services/Controllers/SyncController.cs @@ -24,7 +24,7 @@ namespace CoreSyncServer.Controllers; /// status code as before, plus the X-CoreSync-Error / X-CoreSync-Trace headers and a /// body a current client turns into a typed failure. Anything else /// propagates to the host (exception handler, Sentry) as it always did, tagged with the same headers -/// by . +/// by . No status code differs from before the headers existed. /// [ApiController] [AllowAnonymous] @@ -184,13 +184,14 @@ private ObjectResult SessionExpired(Guid sessionId) => SyncErrorResponses.SessionExpired(HttpContext, sessionId); /// - /// 503 with the agent-unavailable code: the agent owning the data store is not connected or - /// did not answer in time. Transient from the client's point of view. + /// The 503 every agent failure has always produced, logged as it always was: agent-unavailable + /// when the agent owning the data store is not connected or did not answer in time, + /// agent-error when it answered that the operation failed. Both are left retryable. /// private ObjectResult AgentOffline(AgentOfflineException ex) { logger.LogWarning(ex, "Agent offline for DataStore {DataStoreId}", ex.DataStoreId); - return SyncErrorResponses.AgentUnavailable(HttpContext, ex.Message); + return SyncErrorResponses.AgentFailure(HttpContext, ex); } [HttpGet("changes-bulk/{storeId:guid}")] diff --git a/src/Services/Filters/SyncErrorResponses.cs b/src/Services/Filters/SyncErrorResponses.cs index e58fc12..f8941f7 100644 --- a/src/Services/Filters/SyncErrorResponses.cs +++ b/src/Services/Filters/SyncErrorResponses.cs @@ -1,5 +1,6 @@ using CoreSync; using CoreSync.Http; +using CoreSyncServer.Services; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc; @@ -31,6 +32,18 @@ public static ObjectResult SessionExpired(HttpContext httpContext, Guid sessionI public static ObjectResult AgentUnavailable(HttpContext httpContext, string message) => Create(httpContext, StatusCodes.Status503ServiceUnavailable, SyncHttpErrorCodes.AgentUnavailable, message); + /// + /// The 503 an agent failure has always produced, with agent-error when the agent answered + /// with a failure and agent-unavailable when it did not answer at all. Neither code is + /// permanent, so clients keep retrying as they did. + /// + public static ObjectResult AgentFailure(HttpContext httpContext, AgentOfflineException exception) + => Create( + httpContext, + StatusCodes.Status503ServiceUnavailable, + exception is AgentOperationFailedException ? SyncHttpErrorCodes.AgentError : SyncHttpErrorCodes.AgentUnavailable, + exception.Message); + public static ObjectResult ConfigurationError(HttpContext httpContext, string message) => Create(httpContext, StatusCodes.Status422UnprocessableEntity, SyncHttpErrorCodes.ConfigurationError, message); @@ -51,20 +64,27 @@ public static ObjectResult Create(HttpContext httpContext, int statusCode, strin } /// - /// Arranges for the error code and trace headers to be on the response the host produces for an - /// exception this API does not handle, without handling it: the exception keeps propagating to - /// UseExceptionHandler, the Sentry middleware and the request logger exactly as before. + /// Arranges for the error code and trace headers to be on the response the host's exception + /// handler produces for an exception this API does not handle, without handling it: the exception + /// keeps propagating to UseExceptionHandler, the Sentry middleware and the request logger + /// exactly as before. /// /// /// The exception handler middleware clears every header already set when it takes over, so the /// headers are added from an callback, which it does not - /// clear and which runs when its own error response starts. + /// clear and which runs when its error response starts. A host without any handler lets Kestrel + /// write a bare 500, which runs no OnStarting callbacks: that response carries no header, and the + /// status alone still tells the client the server answered. Nothing is registered once the + /// response has started, because that would throw and replace the real failure. /// public static void TagUnhandled(HttpContext httpContext, Exception exception) { + var response = httpContext.Response; + if (response.HasStarted) + return; + var code = Classify(exception); var traceId = httpContext.TraceIdentifier; - var response = httpContext.Response; response.OnStarting(() => { diff --git a/src/Services/Implementation/AgentProxySyncProvider.cs b/src/Services/Implementation/AgentProxySyncProvider.cs index 7f96738..1709557 100644 --- a/src/Services/Implementation/AgentProxySyncProvider.cs +++ b/src/Services/Implementation/AgentProxySyncProvider.cs @@ -1,4 +1,4 @@ -using CoreSync; +using CoreSync; using CoreSyncServer.Agent.Contracts; using CoreSyncServer.Services.Hubs; using Microsoft.AspNetCore.SignalR; @@ -17,7 +17,8 @@ namespace CoreSyncServer.Services.Implementation; /// or the RPC did not complete (timeout, dropped connection), and /// when the agent answered with an error - SignalR /// surfaces the agent handler's exception as a - which is a failure of -/// the data store behind it, not of its availability. +/// the data store behind it, not of its availability. The latter derives from the former, so the +/// controller answers both with the 503 it always did; only the error code header differs. /// /// internal sealed class AgentProxySyncProvider : ISyncProvider @@ -61,7 +62,7 @@ public async Task GetStoreIdAsync(CancellationToken cancellationToken = de } catch (HubException ex) { - // The agent answered: its provider failed. Not an availability problem. + // The agent answered: its provider failed. Same 503 as before, told apart by its code. throw new AgentOperationFailedException(_dataStoreId, "SyncGetStoreId", ex); } catch (Exception ex) when (ex is not AgentOfflineException and not OperationCanceledException) @@ -79,7 +80,7 @@ public async Task GetSyncVersionAsync(CancellationToken cancellatio } catch (HubException ex) { - // The agent answered: its provider failed. Not an availability problem. + // The agent answered: its provider failed. Same 503 as before, told apart by its code. throw new AgentOperationFailedException(_dataStoreId, "SyncGetSyncVersion", ex); } catch (Exception ex) when (ex is not AgentOfflineException and not OperationCanceledException) @@ -108,7 +109,7 @@ public async Task GetChangesAsync( } catch (HubException ex) { - // The agent answered: its provider failed. Not an availability problem. + // The agent answered: its provider failed. Same 503 as before, told apart by its code. throw new AgentOperationFailedException(_dataStoreId, "SyncGetChanges", ex); } catch (Exception ex) when (ex is not AgentOfflineException and not OperationCanceledException) @@ -137,7 +138,7 @@ public async Task ApplyChangesAsync( } catch (HubException ex) { - // The agent answered: its provider failed. Not an availability problem. + // The agent answered: its provider failed. Same 503 as before, told apart by its code. throw new AgentOperationFailedException(_dataStoreId, "SyncApplyChanges", ex); } catch (Exception ex) when (ex is not AgentOfflineException and not OperationCanceledException) @@ -155,7 +156,7 @@ public async Task SaveVersionForStoreAsync(Guid otherStoreId, long version, Canc } catch (HubException ex) { - // The agent answered: its provider failed. Not an availability problem. + // The agent answered: its provider failed. Same 503 as before, told apart by its code. throw new AgentOperationFailedException(_dataStoreId, "SyncSaveVersionForStore", ex); } catch (Exception ex) when (ex is not AgentOfflineException and not OperationCanceledException) diff --git a/tests/CoreSyncServer.Tests/AgentFailureResponseTests.cs b/tests/CoreSyncServer.Tests/AgentFailureResponseTests.cs new file mode 100644 index 0000000..b1fd295 --- /dev/null +++ b/tests/CoreSyncServer.Tests/AgentFailureResponseTests.cs @@ -0,0 +1,52 @@ +using CoreSync.Http; +using CoreSyncServer.Filters; +using CoreSyncServer.Services; +using FluentAssertions; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.SignalR; + +namespace CoreSyncServer.Tests; + +/// +/// An agent that answers a sync RPC with an error and an agent that does not answer at all have +/// always both been a 503 with the exception message as body. They still are - a server deployed +/// ahead of its clients must look identical to them - and the only thing that tells them apart is the +/// error code header a current client reads. +/// +public class AgentFailureResponseTests +{ + [Fact] + public void AgentOperationFailed_IsStillAnAgentOfflineExceptionWithTheLegacyMessage() + { + var ex = new AgentOperationFailedException(12, "SyncApplyChanges", new HubException("FK violation on Posts")); + + // What SyncController catches, and the body text a 1.0.1 client has been receiving + ex.Should().BeAssignableTo(); + ex.Message.Should().Be("Agent RPC 'SyncApplyChanges' failed for DataStore 12."); + ex.DataStoreId.Should().Be(12); + ex.InnerException.Should().BeOfType(); + } + + [Fact] + public void AgentFailure_Keeps503AndTellsTheTwoCasesApartByCode() + { + var reported = new DefaultHttpContext(); + var reportedResult = SyncErrorResponses.AgentFailure(reported, new AgentOperationFailedException(12, "SyncApplyChanges", new HubException("boom"))); + + reportedResult.StatusCode.Should().Be(StatusCodes.Status503ServiceUnavailable); + reported.Response.Headers[SyncHttpHeaders.ErrorCode].ToString().Should().Be(SyncHttpErrorCodes.AgentError); + reported.Response.Headers[SyncHttpHeaders.TraceId].ToString().Should().NotBeNullOrWhiteSpace(); + reportedResult.Value.Should().BeOfType() + .Which.Message.Should().Be("Agent RPC 'SyncApplyChanges' failed for DataStore 12."); + + var offline = new DefaultHttpContext(); + var offlineResult = SyncErrorResponses.AgentFailure(offline, new AgentOfflineException(12, "No agent is currently connected for DataStore 12.")); + + offlineResult.StatusCode.Should().Be(StatusCodes.Status503ServiceUnavailable); + offline.Response.Headers[SyncHttpHeaders.ErrorCode].ToString().Should().Be(SyncHttpErrorCodes.AgentUnavailable); + + // Neither stops a client from retrying, which is what every client did with a 503 + SyncHttpErrorCodes.IsPermanent(SyncHttpErrorCodes.AgentError).Should().BeFalse(); + SyncHttpErrorCodes.IsPermanent(SyncHttpErrorCodes.AgentUnavailable).Should().BeFalse(); + } +} From fe373844787ae87eb566ed42474630a6e03694ba Mon Sep 17 00:00:00 2001 From: Adolfo Marinucci Date: Fri, 16 Oct 2026 18:17:45 +0200 Subject: [PATCH 3/3] Update .NET 10 servicing packages to 10.0.13 and MessagePack to 3.1.12 Microsoft.AspNetCore.Components.WebAssembly.Server 10.0.9 carries CVE-2026-102870 and CVE-2026-96230 (GHSA-g27p-qf6j-c96c, GHSA-qjpx-jqm5-h2c4), fixed in 10.0.13; the rest of the 10.0.9 set moves with it so the Blazor server and client packages stay aligned. MessagePack 3.1.7 is affected by GHSA-qhrr-8q5h-9q3h, fixed in 3.1.9. The NuGet audit fails restore on both with warnings as errors. Bumps CoreSync to 9894eb1 (MessagePack 3.1.12). --- CoreSync | 2 +- src/Agent/CoreSyncServer.Agent.csproj | 6 +++--- src/Data/CoreSyncServer.Data.csproj | 4 ++-- src/Server/CoreSyncServer.csproj | 6 +++--- src/Services/CoreSyncServer.Services.csproj | 6 +++--- src/WebClient/CoreSyncServer.Client.csproj | 6 +++--- tests/CoreSyncServer.Tests/CoreSyncServer.Tests.csproj | 4 ++-- 7 files changed, 17 insertions(+), 17 deletions(-) diff --git a/CoreSync b/CoreSync index 2cc313d..9894eb1 160000 --- a/CoreSync +++ b/CoreSync @@ -1 +1 @@ -Subproject commit 2cc313db0d80f8bd35cab8baf249b17a8cf0be19 +Subproject commit 9894eb1c35e531af1dcc787b4b584109bd9bafc9 diff --git a/src/Agent/CoreSyncServer.Agent.csproj b/src/Agent/CoreSyncServer.Agent.csproj index 5c5ca25..634233f 100644 --- a/src/Agent/CoreSyncServer.Agent.csproj +++ b/src/Agent/CoreSyncServer.Agent.csproj @@ -11,9 +11,9 @@ - - - + + + diff --git a/src/Data/CoreSyncServer.Data.csproj b/src/Data/CoreSyncServer.Data.csproj index 507ea7c..7388c97 100644 --- a/src/Data/CoreSyncServer.Data.csproj +++ b/src/Data/CoreSyncServer.Data.csproj @@ -8,8 +8,8 @@ - - + + diff --git a/src/Server/CoreSyncServer.csproj b/src/Server/CoreSyncServer.csproj index 9e293d1..3b96532 100644 --- a/src/Server/CoreSyncServer.csproj +++ b/src/Server/CoreSyncServer.csproj @@ -18,10 +18,10 @@ - - + + - + runtime; build; native; contentfiles; analyzers; buildtransitive all diff --git a/src/Services/CoreSyncServer.Services.csproj b/src/Services/CoreSyncServer.Services.csproj index 04f0e31..a2123ba 100644 --- a/src/Services/CoreSyncServer.Services.csproj +++ b/src/Services/CoreSyncServer.Services.csproj @@ -12,9 +12,9 @@ - - - + + + diff --git a/src/WebClient/CoreSyncServer.Client.csproj b/src/WebClient/CoreSyncServer.Client.csproj index e59f5c5..ae6c893 100644 --- a/src/WebClient/CoreSyncServer.Client.csproj +++ b/src/WebClient/CoreSyncServer.Client.csproj @@ -12,9 +12,9 @@ - - - + + + diff --git a/tests/CoreSyncServer.Tests/CoreSyncServer.Tests.csproj b/tests/CoreSyncServer.Tests/CoreSyncServer.Tests.csproj index bac1904..35bff09 100644 --- a/tests/CoreSyncServer.Tests/CoreSyncServer.Tests.csproj +++ b/tests/CoreSyncServer.Tests/CoreSyncServer.Tests.csproj @@ -13,11 +13,11 @@ all - + - + runtime; build; native; contentfiles; analyzers; buildtransitive all