diff --git a/src/CoreSync.Http.Client/Implementation/SyncProviderHttpClient.cs b/src/CoreSync.Http.Client/Implementation/SyncProviderHttpClient.cs index 2f212fe..9dfe28f 100644 --- a/src/CoreSync.Http.Client/Implementation/SyncProviderHttpClient.cs +++ b/src/CoreSync.Http.Client/Implementation/SyncProviderHttpClient.cs @@ -32,6 +32,43 @@ public SyncProviderHttpClient(IHttpClientFactory httpClientProvider, SyncProvide _options = options; } + /// + /// Throws for an unsuccessful response, rebuilding from the + /// server's 410 Gone signal so that callers see the same typed error they would from a local provider. + /// + /// + /// Used in place of on every sync endpoint: + /// without it an aged-out anchor arrives as a bare and the only + /// thing an application can tell the user is that synchronization failed. + /// + private static async Task EnsureSyncSuccessAsync(HttpResponseMessage response, CancellationToken cancellationToken) + { + if (response.StatusCode == HttpStatusCode.Gone && + response.Headers.TryGetValues(SyncHttpHeaders.ErrorCode, out var errorCodes) && + errorCodes.Contains(SyncHttpErrorCodes.AnchorTooOld)) + { + SyncAnchorTooOldError? error = null; + try + { + error = await response.Content.ReadFromJsonAsync(cancellationToken); + } + catch (JsonException) + { + // The header alone identifies the condition; a body we cannot read must not downgrade + // this into an ordinary transport failure. + } + + var requestedVersion = error?.RequestedVersion ?? -1; + var minValidVersion = error?.MinValidVersion ?? -1; + + throw error?.TableName is { } tableName + ? new SyncAnchorTooOldException(tableName, requestedVersion, minValidVersion) + : new SyncAnchorTooOldException(requestedVersion, minValidVersion); + } + + response.EnsureSuccessStatusCode(); + } + public async Task ApplyChangesAsync([NotNull] SyncChangeSet changeSet, [CanBeNull] Func? onConflictFunc = null, CancellationToken cancellationToken = default) { var httpClient = _httpClientFactory.CreateClient(_options.HttpClientName ?? Options.DefaultName); @@ -48,8 +85,9 @@ public async Task ApplyChangesAsync([NotNull] SyncChangeSet changeSe SyncProgress?.Invoke(this, new SyncProgressEventArgs(SyncStage.ComputingLocalChanges)); - (await httpClient.PostAsJsonAsync($"/{_options.SyncControllerRoute}/changes-bulk-begin", bulkChangeSet, cancellationToken)) - .EnsureSuccessStatusCode(); + await EnsureSyncSuccessAsync( + await httpClient.PostAsJsonAsync($"/{_options.SyncControllerRoute}/changes-bulk-begin", bulkChangeSet, cancellationToken), + cancellationToken); var listOfItemsToUpload = new List(); @@ -74,8 +112,9 @@ public async Task ApplyChangesAsync([NotNull] SyncChangeSet changeSe beginUploadItemContent.Headers.ContentType = new MediaTypeHeaderValue("application/x-msgpack"); - (await httpClient.PostAsync($"{_options.SyncControllerRoute}/changes-bulk-item-binary", beginUploadItemContent, cancellationToken)) - .EnsureSuccessStatusCode(); + await EnsureSyncSuccessAsync( + await httpClient.PostAsync($"{_options.SyncControllerRoute}/changes-bulk-item-binary", beginUploadItemContent, cancellationToken), + cancellationToken); } else { @@ -85,8 +124,9 @@ public async Task ApplyChangesAsync([NotNull] SyncChangeSet changeSe Items = listOfItemsToUpload }), Encoding.UTF8, "application/json"); - (await httpClient.PostAsync($"{_options.SyncControllerRoute}/changes-bulk-item", beginUploadItemContent, cancellationToken)) - .EnsureSuccessStatusCode(); + await EnsureSyncSuccessAsync( + await httpClient.PostAsync($"{_options.SyncControllerRoute}/changes-bulk-item", beginUploadItemContent, cancellationToken), + cancellationToken); } SyncProgress?.Invoke(this, new SyncProgressEventArgs(SyncStage.ApplyChanges, skip / (double)changeSet.Items.Count)); @@ -94,7 +134,7 @@ public async Task ApplyChangesAsync([NotNull] SyncChangeSet changeSe var remoteChangeSetResponse = await httpClient.PostAsync( $"{_options.SyncControllerRoute}/changes-bulk-complete{(_options.UseBinaryFormat ? "-binary" : string.Empty)}/{sessionId}", null, cancellationToken); - remoteChangeSetResponse.EnsureSuccessStatusCode(); + await EnsureSyncSuccessAsync(remoteChangeSetResponse, cancellationToken); SyncProgress?.Invoke(this, new SyncProgressEventArgs(SyncStage.ApplyChanges, 1.0)); @@ -115,7 +155,7 @@ public async Task GetChangesAsync(Guid otherStoreId, SyncFilterPa } var response = await httpClient.SendAsync(request, cancellationToken); - response.EnsureSuccessStatusCode(); + await EnsureSyncSuccessAsync(response, cancellationToken); var bulkSyncChangeSet = await response.Content.ReadFromJsonAsync(cancellationToken) ?? throw new InvalidOperationException(); @@ -181,7 +221,7 @@ private async Task DownloadBulkChangeSetBinary(BulkSyncChangeSet { var response = await httpClient.GetAsync($"{_options.SyncControllerRoute}/changes-bulk-item-binary/{bulkSyncChangeSet.SessionId}/{skip}/{_options.BulkItemSize}", cancellationToken); - response.EnsureSuccessStatusCode(); + await EnsureSyncSuccessAsync(response, cancellationToken); using var s = await response.Content.ReadAsStreamAsync(cancellationToken); var downloadedItems = await CoreSyncMessagePackSerializer.DeserializeAsync(s, cancellationToken); @@ -189,6 +229,12 @@ private async Task DownloadBulkChangeSetBinary(BulkSyncChangeSet items.AddRange(bulkItems); break; } + catch (SyncAnchorTooOldException) + { + // Permanent for this client: the change history it asked for is gone. Retrying + // burns three round trips and fifteen seconds to reach the same answer. + throw; + } catch (Exception) { if (retry == 0) diff --git a/src/CoreSync.Http.Server/WebApplicationExtensions.cs b/src/CoreSync.Http.Server/WebApplicationExtensions.cs index 6b1826b..180c780 100644 --- a/src/CoreSync.Http.Server/WebApplicationExtensions.cs +++ b/src/CoreSync.Http.Server/WebApplicationExtensions.cs @@ -50,6 +50,9 @@ public static void MapCoreSyncHttpServerEndpoints(this IEndpointRouteBuilder web void ConfigureEndpoint(T endpoint, Action? specificAction = null) where T : IEndpointConventionBuilder { + // Added before the caller's own conventions so that an aged-out anchor reaches the client + // as a documented 410 rather than whatever the host makes of an unhandled exception. + endpoint.AddSyncErrorEndpointFilter(); options.AllEndpoints?.Invoke(endpoint); specificAction?.Invoke(endpoint); } @@ -131,6 +134,45 @@ void ConfigureEndpoint(T endpoint, Action? specificAction = null) where T } +internal static class SyncErrorFilterEndpointExtensions +{ + /// + /// Translates into an HTTP 410 Gone carrying the + /// header and a body. + /// + /// + /// 410 is the honest code here: the change history the client asked for existed once and is + /// permanently gone, so unlike a 5xx there is nothing to retry. Without this the condition + /// surfaced as an unhandled exception, which the client could only report as a generic + /// "unable to synchronize". + /// + public static TBuilder AddSyncErrorEndpointFilter(this TBuilder builder) where TBuilder : IEndpointConventionBuilder + { + builder.AddEndpointFilter(async (context, next) => + { + try + { + return await next(context); + } + catch (SyncAnchorTooOldException ex) + { + context.HttpContext.Response.Headers[SyncHttpHeaders.ErrorCode] = SyncHttpErrorCodes.AnchorTooOld; + + return Results.Json( + new SyncAnchorTooOldError + { + TableName = ex.TableName, + RequestedVersion = ex.RequestedVersion, + MinValidVersion = ex.MinValidVersion + }, + statusCode: StatusCodes.Status410Gone); + } + }); + + return builder; + } +} + internal static class EndpointMessagePackFilterEndpointExtensions { public static TBuilder AddMessagePackEndpointFilter(this TBuilder builder) where TBuilder : IEndpointConventionBuilder diff --git a/src/CoreSync.Http/SyncAnchorTooOldError.cs b/src/CoreSync.Http/SyncAnchorTooOldError.cs new file mode 100644 index 0000000..5616687 --- /dev/null +++ b/src/CoreSync.Http/SyncAnchorTooOldError.cs @@ -0,0 +1,30 @@ +namespace CoreSync.Http; + +/// +/// Body of the HTTP 410 Gone response returned when a client's sync anchor has fallen outside the +/// store's change retention window. +/// +/// +/// The wire form of CoreSync.SyncAnchorTooOldException: the client rebuilds that exception +/// from these values, so the condition reaches application code typed rather than as a generic +/// transport failure. +/// +public class SyncAnchorTooOldError +{ + /// + /// Gets or sets the table whose change history no longer covers , + /// or null when the store keeps a single shared change journal. + /// + public string? TableName { get; set; } + + /// + /// Gets or sets the version the client asked for. A negative value means the server holds no + /// anchor at all for the client. + /// + public long RequestedVersion { get; set; } + + /// + /// Gets or sets the oldest version the server can still resolve changes from. + /// + public long MinValidVersion { get; set; } +} diff --git a/src/CoreSync.Http/SyncHttpErrorCodes.cs b/src/CoreSync.Http/SyncHttpErrorCodes.cs new file mode 100644 index 0000000..ddac2d8 --- /dev/null +++ b/src/CoreSync.Http/SyncHttpErrorCodes.cs @@ -0,0 +1,23 @@ +namespace CoreSync.Http; + +/// +/// Machine-readable values carried by the response header. +/// +/// +/// The header lets a client branch on a failure without parsing the response body or matching on +/// message text, and stays readable even when the body is a message pack payload the client did not +/// expect. +/// +public static class SyncHttpErrorCodes +{ + /// + /// The client asked for changes relative to a version the server can no longer resolve, because + /// the change history covering it has been discarded by the store's retention policy. + /// Accompanied by HTTP 410 Gone and a body. + /// + /// + /// This is permanent for the client involved: retrying with the same anchor can never succeed. + /// The client has to be reinitialized from a fresh snapshot. + /// + public const string AnchorTooOld = "anchor-too-old"; +} diff --git a/src/CoreSync.Http/SyncHttpHeaders.cs b/src/CoreSync.Http/SyncHttpHeaders.cs index 81150bf..1f2f759 100644 --- a/src/CoreSync.Http/SyncHttpHeaders.cs +++ b/src/CoreSync.Http/SyncHttpHeaders.cs @@ -16,4 +16,11 @@ public static class SyncHttpHeaders /// The server uses this to detect truncated headers. /// public const string TablesCount = "X-CoreSync-Tables-Count"; + + /// + /// A machine-readable identifier for a failure the client is expected to handle specifically, + /// set on the response alongside the corresponding status code. + /// See for the defined values. + /// + public const string ErrorCode = "X-CoreSync-Error"; } diff --git a/src/CoreSync.MySql/MySqlSyncProvider.cs b/src/CoreSync.MySql/MySqlSyncProvider.cs index bf8cc24..c967a36 100644 --- a/src/CoreSync.MySql/MySqlSyncProvider.cs +++ b/src/CoreSync.MySql/MySqlSyncProvider.cs @@ -1,4 +1,4 @@ -using JetBrains.Annotations; +using JetBrains.Annotations; using MySqlConnector; using System; using System.Collections.Generic; @@ -299,9 +299,15 @@ public async Task GetChangesAsync(Guid otherStoreId, SyncFilterPa cmd.Parameters.Clear(); var minVersion = await cmd.ExecuteLongScalarAsync(cancellationToken); + // MIN(`id`) is the oldest change still journalled, so the oldest anchor that can still be + // served is the one immediately before it. if (!fromAnchor.IsNull() && fromAnchor.Version < minVersion - 1) { - throw new InvalidOperationException($"Unable to get changes, version of data requested ({fromAnchor}) is too old (min valid version {minVersion})"); + // Traced before throwing: this runs ahead of the per-table loop below, so without + // it a session that fails here leaves no account of why it stopped. + var anchorTooOld = new SyncAnchorTooOldException(fromAnchor.Version, minVersion - 1); + _logger?.Error($"[{_storeId}] {anchorTooOld.Message}"); + throw anchorTooOld; } foreach (var table in tablesToSync.Cast().Where(_ => _.Columns.Any())) diff --git a/src/CoreSync.PostgreSQL/PostgreSQLSyncProvider.cs b/src/CoreSync.PostgreSQL/PostgreSQLSyncProvider.cs index 47c7d52..6d3e04e 100644 --- a/src/CoreSync.PostgreSQL/PostgreSQLSyncProvider.cs +++ b/src/CoreSync.PostgreSQL/PostgreSQLSyncProvider.cs @@ -1,4 +1,4 @@ -using JetBrains.Annotations; +using JetBrains.Annotations; using Npgsql; using System; using System.Collections.Generic; @@ -315,8 +315,16 @@ public async Task GetChangesAsync(Guid otherStoreId, SyncFilterPa cmd.Parameters.Clear(); var minVersion = await cmd.ExecuteLongScalarAsync(cancellationToken); + // MIN(id) is the oldest change still journalled, so the oldest anchor that can still be + // served is the one immediately before it. if (!fromAnchor.IsNull() && fromAnchor.Version < minVersion - 1) - throw new InvalidOperationException($"Unable to get changes, version of data requested ({fromAnchor}) is too old (min valid version {minVersion})"); + { + // Traced before throwing: this runs ahead of the per-table loop below, so without + // it a session that fails here leaves no account of why it stopped. + var anchorTooOld = new SyncAnchorTooOldException(fromAnchor.Version, minVersion - 1); + _logger?.Error($"[{_storeId}] {anchorTooOld.Message}"); + throw anchorTooOld; + } foreach (var table in tablesToSync.Cast().Where(_ => _.Columns.Any())) { diff --git a/src/CoreSync.SqlServer/SqlSyncProvider.cs b/src/CoreSync.SqlServer/SqlSyncProvider.cs index 5f24f56..cb4b1a9 100644 --- a/src/CoreSync.SqlServer/SqlSyncProvider.cs +++ b/src/CoreSync.SqlServer/SqlSyncProvider.cs @@ -730,8 +730,16 @@ public async Task GetChangesAsync(Guid otherStoreId, SyncFilterPa cmd.Parameters.Clear(); var minVersion = await cmd.ExecuteLongScalarAsync(cancellationToken); + // MIN(ID) is the oldest change still journalled, so the oldest anchor that can still be + // served is the one immediately before it. if (!fromAnchor.IsNull() && fromAnchor.Version < minVersion - 1) - throw new InvalidOperationException($"Unable to get changes, version of data requested ({fromAnchor}) is too old (min valid version {minVersion})"); + { + // Traced before throwing: this runs ahead of the per-table loop below, so without + // it a session that fails here leaves no account of why it stopped. + var anchorTooOld = new SyncAnchorTooOldException(fromAnchor.Version, minVersion - 1); + _logger?.Error($"[{_storeId}] {anchorTooOld.Message}"); + throw anchorTooOld; + } foreach (SqlSyncTable table in tablesToSync.Cast()) { diff --git a/src/CoreSync.SqlServerCT/ChangeTrackingDatabaseOptions.cs b/src/CoreSync.SqlServerCT/ChangeTrackingDatabaseOptions.cs index d2b464e..1ad9e99 100644 --- a/src/CoreSync.SqlServerCT/ChangeTrackingDatabaseOptions.cs +++ b/src/CoreSync.SqlServerCT/ChangeTrackingDatabaseOptions.cs @@ -1,10 +1,16 @@ -namespace CoreSync.SqlServerCT +namespace CoreSync.SqlServerCT { /// /// The database-level change tracking settings as reported by sys.change_tracking_databases. /// - internal sealed class ChangeTrackingDatabaseOptions + public sealed class ChangeTrackingDatabaseOptions { + /// + /// Initializes a new instance of the class. + /// + /// The retention period, expressed in . + /// The unit is expressed in. + /// Whether SQL Server automatically removes expired change tracking data. public ChangeTrackingDatabaseOptions(int retentionPeriod, ChangeRetentionUnit retentionPeriodUnit, bool autoCleanup) { RetentionPeriod = retentionPeriod; @@ -12,12 +18,22 @@ public ChangeTrackingDatabaseOptions(int retentionPeriod, ChangeRetentionUnit re AutoCleanup = autoCleanup; } + /// + /// Gets how long change history is kept, expressed in . + /// public int RetentionPeriod { get; } + /// + /// Gets the unit is expressed in. + /// public ChangeRetentionUnit RetentionPeriodUnit { get; } + /// + /// Gets a value indicating whether SQL Server automatically removes expired change tracking data. + /// public bool AutoCleanup { get; } + /// public override string ToString() => $"CHANGE_RETENTION = {RetentionPeriod} {RetentionPeriodUnit}, AUTO_CLEANUP = {(AutoCleanup ? "ON" : "OFF")}"; } } diff --git a/src/CoreSync.SqlServerCT/SqlServerCTProvider.cs b/src/CoreSync.SqlServerCT/SqlServerCTProvider.cs index feffa28..1f1a432 100644 --- a/src/CoreSync.SqlServerCT/SqlServerCTProvider.cs +++ b/src/CoreSync.SqlServerCT/SqlServerCTProvider.cs @@ -283,10 +283,11 @@ public async Task ApplyChangesAsync([NotNull] SyncChangeSet changeSe throw; } } - catch (SyncAnchorTooOldException) + catch (SyncAnchorTooOldException ex) { // Deliberately not wrapped: callers have to be able to tell "reinitialize this client" // apart from a transient error worth retrying. + _logger?.Error($"[{_storeId}] {ex.Message}"); throw; } catch (Exception ex) @@ -555,6 +556,36 @@ [ID] ASC _initialized = true; } + /// + /// Applies the configured change tracking retention to the database and reports the settings + /// actually in effect afterwards, without provisioning anything else. + /// + /// A token to cancel the operation. + /// + /// The settings read back from sys.change_tracking_databases, or null when change + /// tracking is not enabled on the database. + /// + /// + /// Retention is normally reconciled as part of . This exists for + /// callers that let an operator edit the retention on its own: changing the stored setting has no + /// effect until an ALTER DATABASE is actually issued, and a setting that looks saved but was + /// never applied is precisely how a database ends up quietly keeping a week of history when it was + /// configured for a month. The read-back lets the caller show what the database really holds rather + /// than what was requested. + /// + /// + /// The statement failed - most often because the account lacks the ALTER DATABASE permission. + /// + public async Task ApplyChangeRetentionAsync(CancellationToken cancellationToken = default) + { + using var connection = new SqlConnection(Configuration.ConnectionString); + await connection.OpenAsync(cancellationToken); + + await ReconcileChangeTrackingRetentionAsync(connection, cancellationToken); + + return await connection.GetChangeTrackingOptionsAsync(cancellationToken); + } + public async Task ApplyProvisionAsync(CancellationToken cancellationToken = default) { await InitializeStoreAsync(cancellationToken); @@ -758,7 +789,14 @@ public async Task GetChangesAsync(Guid otherStoreId, SyncFilterPa var minVersion = await cmd.ExecuteLongScalarAsync(cancellationToken); if (fromAnchor.Version < minVersion) - throw new SyncAnchorTooOldException(table.NameWithSchema, fromAnchor.Version, minVersion); + { + // Traced before throwing: this runs ahead of the per-table loop below, so + // without it a session that fails here records nothing but its opening + // "Begin GetChanges" line and leaves no account of why it stopped. + var anchorTooOld = new SyncAnchorTooOldException(table.NameWithSchema, fromAnchor.Version, minVersion); + _logger?.Error($"[{_storeId}] {anchorTooOld.Message}"); + throw anchorTooOld; + } } } diff --git a/src/CoreSync.Sqlite/SqliteSyncProvider.cs b/src/CoreSync.Sqlite/SqliteSyncProvider.cs index fcb608c..543cfd4 100644 --- a/src/CoreSync.Sqlite/SqliteSyncProvider.cs +++ b/src/CoreSync.Sqlite/SqliteSyncProvider.cs @@ -460,8 +460,16 @@ public async Task GetChangesAsync(Guid otherStoreId, SyncFilterPa cmd.Parameters.Clear(); var minVersion = await cmd.ExecuteLongScalarAsync(cancellationToken); + // MIN(ID) is the oldest change still journalled, so the oldest anchor that can still be + // served is the one immediately before it. if (!fromAnchor.IsNull() && fromAnchor.Version < minVersion - 1) - throw new InvalidOperationException($"Unable to get changes, version of data requested ({fromAnchor}) is too old (min valid version {minVersion})"); + { + // Traced before throwing: this runs ahead of the per-table loop below, so without + // it a session that fails here leaves no account of why it stopped. + var anchorTooOld = new SyncAnchorTooOldException(fromAnchor.Version, minVersion - 1); + _logger?.Error($"[{_storeId}] {anchorTooOld.Message}"); + throw anchorTooOld; + } foreach (var table in tablesToSync.Cast().Where(_ => _.Columns.Any())) { diff --git a/src/CoreSync.Tests/AnchorTooOldTests.cs b/src/CoreSync.Tests/AnchorTooOldTests.cs new file mode 100644 index 0000000..c7f6a34 --- /dev/null +++ b/src/CoreSync.Tests/AnchorTooOldTests.cs @@ -0,0 +1,308 @@ +using CoreSync.Http; +using CoreSync.Sqlite; +using CoreSync.Tests.Data; +using Microsoft.Data.Sqlite; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using Shouldly; +using SQLite; +using System; +using System.Collections.Generic; +using System.IO; +using System.Linq; +using System.Net; +using System.Net.Http; +using System.Net.Http.Json; +using System.Threading; +using System.Threading.Tasks; + +namespace CoreSync.Tests; + +/// +/// Covers the aged-out anchor path: a peer asking for changes relative to a version the store no +/// longer retains must get a typed , and that condition must +/// survive the trip over HTTP instead of collapsing into a generic transport failure. +/// +/// +/// This is the failure behind "synchronization rarely works": any client idle longer than the change +/// retention window asks for a version that has been cleaned up. It is permanent for that client - +/// the only cure is a fresh snapshot - so what matters is that callers can *recognise* it rather than +/// see an anonymous error they are tempted to retry forever. +/// +[TestClass] +public class AnchorTooOldTests +{ + private static string CreateDatabase(string testName) + { + var dbFile = Path.Combine(Path.GetTempPath(), $"{testName}_{Guid.NewGuid():N}.sqlite"); + + using (var db = new SQLiteConnection(dbFile)) + { + db.CreateTable(); + } + + SqliteConnection.ClearAllPools(); + + return dbFile; + } + + private static SqliteSyncProvider CreateProvider(string dbFile) + => new(new SqliteSyncConfigurationBuilder($"Data Source={dbFile}") + .Table() + .Build(), logger: new ConsoleLogger("LOC")); + + [TestMethod] + public async Task GetChangesAsync_ThrowsTypedExceptionWhenAnchorFellOutOfRetention() + { + var dbFile = CreateDatabase(nameof(GetChangesAsync_ThrowsTypedExceptionWhenAnchorFellOutOfRetention)); + var syncProvider = CreateProvider(dbFile); + + await syncProvider.ApplyProvisionAsync(); + + using (var db = new SQLiteConnection(dbFile)) + { + for (var i = 0; i < 10; i++) + { + db.Insert(new Stock { Id = Guid.NewGuid(), Symbol = $"SYM{i}" }); + } + } + + SqliteConnection.ClearAllPools(); + + var otherStoreId = Guid.NewGuid(); + + //the peer last synchronized at version 1 and then went quiet + await syncProvider.SaveVersionForStoreAsync(otherStoreId, 1); + + //retention cleanup discards everything below version 5, exactly like SQL Server change + //tracking auto-cleanup does once CHANGE_RETENTION elapses + await syncProvider.ApplyRetentionPolicyAsync(5); + + var ex = await Should.ThrowAsync( + () => syncProvider.GetChangesAsync(otherStoreId, syncDirection: SyncDirection.DownloadOnly)); + + ex.RequestedVersion.ShouldBe(1); + + //the oldest retained change is version 5, so the oldest anchor still resolvable is 4 + ex.MinValidVersion.ShouldBe(4); + + //this store keeps one shared journal, so no individual table is implicated + ex.TableName.ShouldBeNull(); + + //the message has to name both versions: it is the only diagnostic that reaches the operator + ex.Message.ShouldContain("1"); + ex.Message.ShouldContain("4"); + } + + [TestMethod] + public async Task GetChangesAsync_SucceedsWhenAnchorIsStillWithinRetention() + { + var dbFile = CreateDatabase(nameof(GetChangesAsync_SucceedsWhenAnchorIsStillWithinRetention)); + var syncProvider = CreateProvider(dbFile); + + await syncProvider.ApplyProvisionAsync(); + + using (var db = new SQLiteConnection(dbFile)) + { + for (var i = 0; i < 10; i++) + { + db.Insert(new Stock { Id = Guid.NewGuid(), Symbol = $"SYM{i}" }); + } + } + + SqliteConnection.ClearAllPools(); + + var otherStoreId = Guid.NewGuid(); + + //anchor sits exactly on the retention boundary: the oldest retained change is 5, so an + //anchor of 4 is still resolvable and must not be rejected + await syncProvider.SaveVersionForStoreAsync(otherStoreId, 4); + await syncProvider.ApplyRetentionPolicyAsync(5); + + var changeSet = await syncProvider.GetChangesAsync(otherStoreId, syncDirection: SyncDirection.DownloadOnly); + + changeSet.ShouldNotBeNull(); + } + + [TestMethod] + public async Task SyncAgent_WrapsTheTypedExceptionSoExistingCatchesStillFire() + { + var localFile = CreateDatabase(nameof(SyncAgent_WrapsTheTypedExceptionSoExistingCatchesStillFire) + "_local"); + var remoteFile = CreateDatabase(nameof(SyncAgent_WrapsTheTypedExceptionSoExistingCatchesStillFire) + "_remote"); + + var local = CreateProvider(localFile); + var remote = CreateProvider(remoteFile); + + await local.ApplyProvisionAsync(); + await remote.ApplyProvisionAsync(); + + using (var db = new SQLiteConnection(remoteFile)) + { + for (var i = 0; i < 10; i++) + { + db.Insert(new Stock { Id = Guid.NewGuid(), Symbol = $"SYM{i}" }); + } + } + + SqliteConnection.ClearAllPools(); + + //the remote store believes this client is at version 1, then loses everything below 5 + await remote.SaveVersionForStoreAsync(await local.GetStoreIdAsync(), 1); + await remote.ApplyRetentionPolicyAsync(5); + + var agent = new SyncAgent(local, remote); + + // Callers have caught SynchronizationException around SynchronizeAsync since before the typed + // exception existed. Letting this one case escape unwrapped would break them at runtime with + // nothing to catch it at compile time, so every failure still leaves here wrapped. + var ex = await Should.ThrowAsync(() => agent.SynchronizeAsync()); + + ex.InnerException.ShouldBeOfType(); + + var tooOld = (SyncAnchorTooOldException)ex.InnerException!; + tooOld.RequestedVersion.ShouldBe(1); + tooOld.MinValidVersion.ShouldBe(4); + } + + [TestMethod] + public async Task HttpClient_RebuildsTypedExceptionFromServerSignal() + { + var thrown = new SyncAnchorTooOldException("[admin].[UserRole]", 1378, 1925); + using var server = SyncTestServer.Create(new ThrowingSyncProvider(thrown)); + + var ex = await Should.ThrowAsync( + () => server.HttpSyncProvider.GetChangesAsync(Guid.NewGuid(), syncDirection: SyncDirection.DownloadOnly)); + + //every field the caller needs to diagnose the failure has to survive the wire + ex.TableName.ShouldBe("[admin].[UserRole]"); + ex.RequestedVersion.ShouldBe(1378); + ex.MinValidVersion.ShouldBe(1925); + } + + [TestMethod] + public async Task HttpClient_RebuildsTypedExceptionForAStoreWideFailure() + { + var thrown = new SyncAnchorTooOldException(1378, 1925); + using var server = SyncTestServer.Create(new ThrowingSyncProvider(thrown)); + + var ex = await Should.ThrowAsync( + () => server.HttpSyncProvider.GetChangesAsync(Guid.NewGuid(), syncDirection: SyncDirection.DownloadOnly)); + + ex.TableName.ShouldBeNull(); + ex.RequestedVersion.ShouldBe(1378); + ex.MinValidVersion.ShouldBe(1925); + } + + [TestMethod] + public async Task HttpClient_RebuildsTypedExceptionRaisedWhileApplyingChanges() + { + var thrown = new SyncAnchorTooOldException("[core].[Survey]", 1378, 1925); + using var server = SyncTestServer.Create(new ThrowingSyncProvider(thrown)); + + //the upload path guards the anchor too, and has to report it just as distinguishably + var changeSet = new SyncChangeSet( + new SyncAnchor(Guid.NewGuid(), 1378), + SyncAnchor.Null, + new List()); + + var ex = await Should.ThrowAsync( + () => server.HttpSyncProvider.ApplyChangesAsync(changeSet)); + + ex.TableName.ShouldBe("[core].[Survey]"); + ex.RequestedVersion.ShouldBe(1378); + ex.MinValidVersion.ShouldBe(1925); + } + + [TestMethod] + public async Task HttpServer_RespondsWithGoneAndAMachineReadableCode() + { + var thrown = new SyncAnchorTooOldException("[ref].[AgeClass]", 2107, 2120); + using var server = SyncTestServer.Create(new ThrowingSyncProvider(thrown)); + + using var response = await server.RawHttpClient.GetAsync( + $"/api/sync-agent/changes-bulk/{Guid.NewGuid()}"); + + //410 rather than 500: the requested history existed once and is permanently gone, so a + //client that retries it can never succeed + response.StatusCode.ShouldBe(HttpStatusCode.Gone); + + response.Headers.GetValues(SyncHttpHeaders.ErrorCode) + .ShouldContain(SyncHttpErrorCodes.AnchorTooOld); + + var error = await response.Content.ReadFromJsonAsync(); + + error.ShouldNotBeNull(); + error!.TableName.ShouldBe("[ref].[AgeClass]"); + error.RequestedVersion.ShouldBe(2107); + error.MinValidVersion.ShouldBe(2120); + } + + [TestMethod] + public async Task HttpClient_LeavesUnrelatedFailuresAlone() + { + using var server = SyncTestServer.Create( + new ThrowingSyncProvider(new InvalidOperationException("something else went wrong"))); + + //an ordinary server-side error must not be mistaken for an aged-out anchor, which would tell + //the operator to reinitialize a client that has nothing wrong with it + var ex = await Should.ThrowAsync( + () => server.HttpSyncProvider.GetChangesAsync(Guid.NewGuid(), syncDirection: SyncDirection.DownloadOnly)); + + ex.ShouldNotBeAssignableTo(); + } + + [TestMethod] + public async Task HttpServer_LeavesUnrelatedFailuresWithoutTheErrorHeader() + { + using var server = SyncTestServer.Create( + new ThrowingSyncProvider(new InvalidOperationException("something else went wrong"))); + + //the filter must only claim the failures it actually recognises + var ex = await Should.ThrowAsync( + () => server.RawHttpClient.GetAsync($"/api/sync-agent/changes-bulk/{Guid.NewGuid()}")); + + ex.ShouldNotBeAssignableTo(); + } + + /// + /// A provider whose only job is to fail in a specific way, so the HTTP layer can be tested + /// independently of any particular database. + /// + private sealed class ThrowingSyncProvider : ISyncProvider + { + private readonly Exception _exception; + + public ThrowingSyncProvider(Exception exception) => _exception = exception; + + public string[]? SyncTableNames => null; + + public Task GetChangesAsync(Guid otherStoreId, SyncFilterParameter[]? syncFilterParameters = null, SyncDirection syncDirection = SyncDirection.UploadAndDownload, string[]? tables = null, CancellationToken cancellationToken = default) + => throw _exception; + + public Task ApplyChangesAsync(SyncChangeSet changeSet, Func? onConflictFunc = null, CancellationToken cancellationToken = default) + => throw _exception; + + public Task GetStoreIdAsync(CancellationToken cancellationToken = default) + => Task.FromResult(Guid.NewGuid()); + + public Task SaveVersionForStoreAsync(Guid otherStoreId, long version, CancellationToken cancellationToken = default) + => Task.CompletedTask; + + public Task ApplyProvisionAsync(CancellationToken cancellationToken = default) + => Task.CompletedTask; + + public Task RemoveProvisionAsync(CancellationToken cancellationToken = default) + => Task.CompletedTask; + + public Task GetSyncVersionAsync(CancellationToken cancellationToken = default) + => throw new NotSupportedException(); + + public Task ApplyRetentionPolicyAsync(int minVersion, CancellationToken cancellationToken = default) + => throw new NotSupportedException(); + + public Task EnableChangeTrackingForTable(string tableName, CancellationToken cancellationToken = default) + => Task.CompletedTask; + + public Task DisableChangeTrackingForTable(string tableName, CancellationToken cancellationToken = default) + => Task.CompletedTask; + } +} diff --git a/src/CoreSync.Tests/SqlServerCTResilienceTests.cs b/src/CoreSync.Tests/SqlServerCTResilienceTests.cs index ce48da8..a0067f6 100644 --- a/src/CoreSync.Tests/SqlServerCTResilienceTests.cs +++ b/src/CoreSync.Tests/SqlServerCTResilienceTests.cs @@ -211,7 +211,7 @@ await RunInDatabase("CoreSyncCT_UpdateNullAnchor", async connStr => var ex = await Should.ThrowAsync(() => provider.ApplyChangesAsync(changeSet)); - ex.TableName.ShouldContain("Authors"); + ex.TableName!.ShouldContain("Authors"); ex.RequestedVersion.ShouldBe(-1); ex.MinValidVersion.ShouldBeGreaterThanOrEqualTo(0); @@ -253,7 +253,7 @@ await RunInDatabase("CoreSyncCT_StaleAnchor", async connStr => var ex = await Should.ThrowAsync(() => provider.ApplyChangesAsync(changeSet)); - ex.TableName.ShouldContain("Authors"); + ex.TableName!.ShouldContain("Authors"); ex.RequestedVersion.ShouldBe(0); ex.MinValidVersion.ShouldBeGreaterThan(0); diff --git a/src/CoreSync.Tests/SyncTestServer.cs b/src/CoreSync.Tests/SyncTestServer.cs index 7bb31c1..03eeee1 100644 --- a/src/CoreSync.Tests/SyncTestServer.cs +++ b/src/CoreSync.Tests/SyncTestServer.cs @@ -22,6 +22,12 @@ internal sealed class SyncTestServer : IDisposable public ISyncProviderHttpClient HttpSyncProvider { get; } + /// + /// The underlying , for tests that need to assert on the raw response + /// (status code, headers, body) rather than on what the sync provider makes of it. + /// + public HttpClient RawHttpClient => _httpClient; + private SyncTestServer(WebApplication app, HttpClient httpClient, ServiceProvider clientServiceProvider, ISyncProviderHttpClient httpSyncProvider) { _app = app; diff --git a/src/CoreSync/SyncAgent.cs b/src/CoreSync/SyncAgent.cs index 1994a2c..d3bb615 100644 --- a/src/CoreSync/SyncAgent.cs +++ b/src/CoreSync/SyncAgent.cs @@ -61,7 +61,23 @@ public SyncAgent(ISyncProviderBase localSyncProvider, ISyncProviderBase remoteSy /// Optional filter parameters applied when retrieving changes from the local provider. /// /// A task that represents the asynchronous synchronization operation. - /// Thrown when synchronization fails. + /// + /// Thrown when synchronization fails, with the underlying failure as + /// . + /// + /// An inner means one of the stores can no longer + /// resolve changes from the other's anchor, because the change history covering it has been + /// discarded by its retention policy. That is permanent: the affected store has to be + /// reinitialized from a fresh snapshot, and retrying will not help. + /// + /// catch (SynchronizationException ex) + /// when (ex.InnerException is SyncAnchorTooOldException tooOld) + /// { + /// // tooOld.TableName, tooOld.RequestedVersion, tooOld.MinValidVersion + /// } + /// + /// + /// public async Task SynchronizeAsync( Func? remoteConflictResolutionFunc = null, Func? localConflictResolutionFunc = null, @@ -88,15 +104,14 @@ public async Task SynchronizeAsync( //await LocalSyncProvider.ApplyProvisionAsync(cancellationToken: cancellationToken); //await RemoteSyncProvider.ApplyProvisionAsync(cancellationToken: cancellationToken); } - catch (SyncAnchorTooOldException) - { - // Deliberately not wrapped: this one is not retryable, the client has to be - // reinitialized from a fresh snapshot, and callers need to see that without digging - // through inner exceptions. - throw; - } catch (Exception ex) { + // Every failure leaves here as a SynchronizationException, including + // SyncAnchorTooOldException: callers have caught this one type around + // SynchronizeAsync since before the typed exception existed, and letting one case + // escape unwrapped would break them at runtime with nothing to catch it at compile + // time. The typed exception stays reachable as the inner exception, which is what + // callers branch on to tell "reinitialize this client" from a transient failure. throw new SynchronizationException("Unable to synchronize stores", ex); } } diff --git a/src/CoreSync/SyncAnchorTooOldException.cs b/src/CoreSync/SyncAnchorTooOldException.cs index 2fe64f2..b105954 100644 --- a/src/CoreSync/SyncAnchorTooOldException.cs +++ b/src/CoreSync/SyncAnchorTooOldException.cs @@ -19,6 +19,11 @@ namespace CoreSync /// It derives from so that callers written against the /// previous, untyped behaviour keep working. /// + /// + /// Providers raise it directly. wraps it, like every + /// other failure, in a - so callers at that level branch + /// on rather than catching this type directly. + /// /// public class SyncAnchorTooOldException : InvalidOperationException { @@ -56,9 +61,27 @@ public SyncAnchorTooOldException(string tableName, long requestedVersion, long m } /// - /// Gets the name of the table whose change history no longer covers . + /// Initializes a new instance of the class for a store + /// that tracks changes in a single shared journal, where no individual table is implicated. /// - public string TableName { get; } + /// + /// The version the operation asked for. A negative value means no anchor was recorded at all + /// (see ), which is equally unrecoverable. + /// + /// The oldest version the store can still resolve changes from. + public SyncAnchorTooOldException(long requestedVersion, long minValidVersion) + : base(BuildMessage(null, requestedVersion, minValidVersion)) + { + RequestedVersion = requestedVersion; + MinValidVersion = minValidVersion; + } + + /// + /// Gets the name of the table whose change history no longer covers , + /// or null when the store keeps a single shared change journal and no individual table is + /// implicated. + /// + public string? TableName { get; } /// /// Gets the version the operation asked for. A negative value means the store holds no anchor @@ -71,13 +94,17 @@ public SyncAnchorTooOldException(string tableName, long requestedVersion, long m /// public long MinValidVersion { get; } - private static string BuildMessage(string tableName, long requestedVersion, long minValidVersion) + private static string BuildMessage(string? tableName, long requestedVersion, long minValidVersion) { var requested = requestedVersion < 0 ? "no anchor is recorded for the peer store" : $"version {requestedVersion} is older than the retained history"; - return $"Unable to resolve changes for table '{tableName}': {requested} " + + var target = tableName == null + ? "Unable to resolve changes" + : $"Unable to resolve changes for table '{tableName}'"; + + return $"{target}: {requested} " + $"(requested version {requestedVersion}, minimum valid version {minValidVersion}). " + "The peer store must be reinitialized from a fresh snapshot; retrying will not help."; }