better 404 handling.
This commit is contained in:
@@ -82,6 +82,13 @@ public class SynchronizerErrorContext
|
|||||||
/// </summary>
|
/// </summary>
|
||||||
public string OperationType { get; set; }
|
public string OperationType { get; set; }
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// Gets or sets whether the error was explicitly classified as a missing remote entity.
|
||||||
|
/// This is used to distinguish true "mail/folder/event no longer exists" cases from
|
||||||
|
/// unrelated HTTP 404 responses that should still surface to the user.
|
||||||
|
/// </summary>
|
||||||
|
public bool IsEntityNotFound { get; set; }
|
||||||
|
|
||||||
/// <summary>
|
/// <summary>
|
||||||
/// Gets whether this error should be retried based on severity and retry count.
|
/// Gets whether this error should be retried based on severity and retry count.
|
||||||
/// </summary>
|
/// </summary>
|
||||||
|
|||||||
@@ -5,6 +5,7 @@ using FluentAssertions;
|
|||||||
using Google.Apis.Requests;
|
using Google.Apis.Requests;
|
||||||
using Moq;
|
using Moq;
|
||||||
using Wino.Core.Domain.Entities.Mail;
|
using Wino.Core.Domain.Entities.Mail;
|
||||||
|
using Wino.Core.Domain.Exceptions;
|
||||||
using Wino.Core.Domain.Entities.Shared;
|
using Wino.Core.Domain.Entities.Shared;
|
||||||
using Wino.Core.Domain.Interfaces;
|
using Wino.Core.Domain.Interfaces;
|
||||||
using Wino.Core.Domain.Models.Synchronization;
|
using Wino.Core.Domain.Models.Synchronization;
|
||||||
@@ -142,6 +143,70 @@ public sealed class GmailSynchronizerRequestSuccessTests
|
|||||||
errorFactory.Verify(x => x.HandleErrorAsync(It.IsAny<SynchronizerErrorContext>()), Times.Once);
|
errorFactory.Verify(x => x.HandleErrorAsync(It.IsAny<SynchronizerErrorContext>()), Times.Once);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public async Task ProcessSingleNativeRequestResponseAsync_Generic404Error_DoesNotClassifyAsEntityNotFound()
|
||||||
|
{
|
||||||
|
var changeProcessor = new Mock<IGmailChangeProcessor>(MockBehavior.Strict);
|
||||||
|
SynchronizerErrorContext? capturedContext = null;
|
||||||
|
|
||||||
|
var errorFactory = new Mock<IGmailSynchronizerErrorHandlerFactory>(MockBehavior.Strict);
|
||||||
|
errorFactory
|
||||||
|
.Setup(x => x.HandleErrorAsync(It.IsAny<SynchronizerErrorContext>()))
|
||||||
|
.Callback<SynchronizerErrorContext>(context => capturedContext = context)
|
||||||
|
.ReturnsAsync(false);
|
||||||
|
|
||||||
|
var synchronizer = CreateSynchronizer(changeProcessor.Object, errorFactory.Object);
|
||||||
|
var request = new DeleteRequest(CreateMailCopy("mail-1"));
|
||||||
|
var bundle = new HttpRequestBundle<IClientServiceRequest>(Mock.Of<IClientServiceRequest>(), request, request);
|
||||||
|
using var response = new HttpResponseMessage(HttpStatusCode.OK)
|
||||||
|
{
|
||||||
|
Content = new StringContent(string.Empty)
|
||||||
|
};
|
||||||
|
var error = new RequestError
|
||||||
|
{
|
||||||
|
Code = 404,
|
||||||
|
Message = "Not Found"
|
||||||
|
};
|
||||||
|
|
||||||
|
var act = () => InvokeProcessSingleNativeRequestResponseAsync(synchronizer, bundle, response, error);
|
||||||
|
|
||||||
|
await act.Should().ThrowAsync<SynchronizerException>();
|
||||||
|
capturedContext.Should().NotBeNull();
|
||||||
|
capturedContext!.IsEntityNotFound.Should().BeFalse();
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public async Task ProcessSingleNativeRequestResponseAsync_Entity404Error_ClassifiesAsEntityNotFound()
|
||||||
|
{
|
||||||
|
var changeProcessor = new Mock<IGmailChangeProcessor>(MockBehavior.Strict);
|
||||||
|
SynchronizerErrorContext? capturedContext = null;
|
||||||
|
|
||||||
|
var errorFactory = new Mock<IGmailSynchronizerErrorHandlerFactory>(MockBehavior.Strict);
|
||||||
|
errorFactory
|
||||||
|
.Setup(x => x.HandleErrorAsync(It.IsAny<SynchronizerErrorContext>()))
|
||||||
|
.Callback<SynchronizerErrorContext>(context => capturedContext = context)
|
||||||
|
.ReturnsAsync(false);
|
||||||
|
|
||||||
|
var synchronizer = CreateSynchronizer(changeProcessor.Object, errorFactory.Object);
|
||||||
|
var request = new DeleteRequest(CreateMailCopy("mail-1"));
|
||||||
|
var bundle = new HttpRequestBundle<IClientServiceRequest>(Mock.Of<IClientServiceRequest>(), request, request);
|
||||||
|
using var response = new HttpResponseMessage(HttpStatusCode.OK)
|
||||||
|
{
|
||||||
|
Content = new StringContent(string.Empty)
|
||||||
|
};
|
||||||
|
var error = new RequestError
|
||||||
|
{
|
||||||
|
Code = 404,
|
||||||
|
Message = "Requested entity was not found."
|
||||||
|
};
|
||||||
|
|
||||||
|
var act = () => InvokeProcessSingleNativeRequestResponseAsync(synchronizer, bundle, response, error);
|
||||||
|
|
||||||
|
await act.Should().ThrowAsync<SynchronizerEntityNotFoundException>();
|
||||||
|
capturedContext.Should().NotBeNull();
|
||||||
|
capturedContext!.IsEntityNotFound.Should().BeTrue();
|
||||||
|
}
|
||||||
|
|
||||||
private static GmailSynchronizer CreateSynchronizer(
|
private static GmailSynchronizer CreateSynchronizer(
|
||||||
IGmailChangeProcessor changeProcessor,
|
IGmailChangeProcessor changeProcessor,
|
||||||
IGmailSynchronizerErrorHandlerFactory? errorFactory = null)
|
IGmailSynchronizerErrorHandlerFactory? errorFactory = null)
|
||||||
|
|||||||
@@ -21,7 +21,7 @@ public class GmailSynchronizerErrorHandlingFactory : SynchronizerErrorHandlingFa
|
|||||||
RegisterHandler(authenticationFailedHandler);
|
RegisterHandler(authenticationFailedHandler);
|
||||||
RegisterHandler(quotaExceededHandler);
|
RegisterHandler(quotaExceededHandler);
|
||||||
RegisterHandler(historyExpiredHandler);
|
RegisterHandler(historyExpiredHandler);
|
||||||
|
RegisterHandler(rateLimitHandler);
|
||||||
RegisterHandler(entityNotFoundHandler);
|
RegisterHandler(entityNotFoundHandler);
|
||||||
RegisterHandler(rateLimitHandler); // Most generic rate limit handler last
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -8,26 +8,27 @@ using Wino.Core.Domain.Models.Synchronization;
|
|||||||
namespace Wino.Core.Synchronizers.Errors;
|
namespace Wino.Core.Synchronizers.Errors;
|
||||||
|
|
||||||
/// <summary>
|
/// <summary>
|
||||||
/// Generic handler for 404 (Not Found) errors across all synchronizers.
|
/// Handles errors that were explicitly classified as missing remote entities.
|
||||||
/// When a resource is already gone on the server, this handler applies
|
/// This avoids swallowing unrelated HTTP 404 responses that should still surface
|
||||||
/// the intended change locally instead of throwing.
|
/// to the user as real synchronization failures.
|
||||||
/// Works for all mail actions, folder actions, and batch operations.
|
|
||||||
/// </summary>
|
/// </summary>
|
||||||
public class EntityNotFoundHandler : ISynchronizerErrorHandler
|
public class EntityNotFoundHandler : ISynchronizerErrorHandler
|
||||||
{
|
{
|
||||||
private readonly ILogger _logger = Log.ForContext<EntityNotFoundHandler>();
|
private readonly ILogger _logger = Log.ForContext<EntityNotFoundHandler>();
|
||||||
private readonly IMailService _mailService;
|
private readonly IMailService _mailService;
|
||||||
private readonly IFolderService _folderService;
|
private readonly IFolderService _folderService;
|
||||||
|
private readonly ICalendarService _calendarService;
|
||||||
|
|
||||||
public EntityNotFoundHandler(IMailService mailService, IFolderService folderService)
|
public EntityNotFoundHandler(IMailService mailService, IFolderService folderService, ICalendarService calendarService)
|
||||||
{
|
{
|
||||||
_mailService = mailService;
|
_mailService = mailService;
|
||||||
_folderService = folderService;
|
_folderService = folderService;
|
||||||
|
_calendarService = calendarService;
|
||||||
}
|
}
|
||||||
|
|
||||||
public bool CanHandle(SynchronizerErrorContext error)
|
public bool CanHandle(SynchronizerErrorContext error)
|
||||||
{
|
{
|
||||||
if (error.ErrorCode != 404) return false;
|
if (!error.IsEntityNotFound) return false;
|
||||||
if (error.RequestBundle == null) return false;
|
if (error.RequestBundle == null) return false;
|
||||||
return true;
|
return true;
|
||||||
}
|
}
|
||||||
@@ -81,9 +82,30 @@ public class EntityNotFoundHandler : ISynchronizerErrorHandler
|
|||||||
return true;
|
return true;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// --- Individual calendar actions ---
|
||||||
|
if (uiRequest is ICalendarActionRequest calendarAction)
|
||||||
|
{
|
||||||
|
_logger.Warning("Entity not found for calendar operation {Op} on {CalendarItemId}. Deleting locally.",
|
||||||
|
calendarAction.Operation, calendarAction.Item?.Id);
|
||||||
|
|
||||||
|
try
|
||||||
|
{
|
||||||
|
if (calendarAction.Item != null)
|
||||||
|
{
|
||||||
|
await _calendarService.DeleteCalendarItemAsync(calendarAction.Item.Id).ConfigureAwait(false);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
catch (Exception ex)
|
||||||
|
{
|
||||||
|
_logger.Error(ex, "Failed to delete calendar item locally after entity-not-found.");
|
||||||
|
}
|
||||||
|
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
|
||||||
// --- Batch requests (can't identify specific item) ---
|
// --- Batch requests (can't identify specific item) ---
|
||||||
// Mark as recoverable. Next sync will clean up stale items.
|
// Mark as recoverable. Next sync will clean up stale items.
|
||||||
_logger.Warning("Entity not found (404) for batch operation. Marking as recoverable.");
|
_logger.Warning("Entity not found for batch operation. Marking as recoverable.");
|
||||||
return true;
|
return true;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -1665,6 +1665,8 @@ public class GmailSynchronizer : WinoSynchronizer<IClientServiceRequest, Message
|
|||||||
{
|
{
|
||||||
if (error == null) return;
|
if (error == null) return;
|
||||||
|
|
||||||
|
var isEntityNotFound = IsKnownGmailEntityNotFoundError(error, bundle);
|
||||||
|
|
||||||
// Create error context
|
// Create error context
|
||||||
var errorContext = new SynchronizerErrorContext
|
var errorContext = new SynchronizerErrorContext
|
||||||
{
|
{
|
||||||
@@ -1672,6 +1674,7 @@ public class GmailSynchronizer : WinoSynchronizer<IClientServiceRequest, Message
|
|||||||
ErrorCode = error.Code,
|
ErrorCode = error.Code,
|
||||||
ErrorMessage = error.Message,
|
ErrorMessage = error.Message,
|
||||||
RequestBundle = bundle,
|
RequestBundle = bundle,
|
||||||
|
IsEntityNotFound = isEntityNotFound,
|
||||||
AdditionalData = new Dictionary<string, object>
|
AdditionalData = new Dictionary<string, object>
|
||||||
{
|
{
|
||||||
{ "Error", error }
|
{ "Error", error }
|
||||||
@@ -1702,7 +1705,7 @@ public class GmailSynchronizer : WinoSynchronizer<IClientServiceRequest, Message
|
|||||||
}
|
}
|
||||||
|
|
||||||
// Entity not found.
|
// Entity not found.
|
||||||
if (error.Code == 404)
|
if (isEntityNotFound)
|
||||||
{
|
{
|
||||||
bundle?.UIChangeRequest?.RevertUIChanges();
|
bundle?.UIChangeRequest?.RevertUIChanges();
|
||||||
throw new SynchronizerEntityNotFoundException(error.Message);
|
throw new SynchronizerEntityNotFoundException(error.Message);
|
||||||
@@ -1718,6 +1721,50 @@ public class GmailSynchronizer : WinoSynchronizer<IClientServiceRequest, Message
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private static bool IsKnownGmailEntityNotFoundError(
|
||||||
|
RequestError error,
|
||||||
|
IRequestBundle<IClientServiceRequest> bundle)
|
||||||
|
{
|
||||||
|
if (error?.Code != 404 || bundle?.UIChangeRequest == null)
|
||||||
|
return false;
|
||||||
|
|
||||||
|
if (!IsExistingEntityOperation(bundle.UIChangeRequest))
|
||||||
|
return false;
|
||||||
|
|
||||||
|
var message = error.Message?.Trim() ?? string.Empty;
|
||||||
|
if (string.IsNullOrWhiteSpace(message))
|
||||||
|
return false;
|
||||||
|
|
||||||
|
var normalizedMessage = message.ToLowerInvariant();
|
||||||
|
return normalizedMessage.Contains("requested entity")
|
||||||
|
|| normalizedMessage.Contains("message not found")
|
||||||
|
|| normalizedMessage.Contains("thread not found")
|
||||||
|
|| normalizedMessage.Contains("draft not found")
|
||||||
|
|| normalizedMessage.Contains("label not found")
|
||||||
|
|| normalizedMessage.Contains("event not found")
|
||||||
|
|| normalizedMessage.Contains("calendar not found");
|
||||||
|
}
|
||||||
|
|
||||||
|
private static bool IsExistingEntityOperation(IUIChangeRequest request)
|
||||||
|
=> request is BatchDeleteRequest
|
||||||
|
|| request is BatchMoveRequest
|
||||||
|
|| request is BatchChangeFlagRequest
|
||||||
|
|| request is BatchMarkReadRequest
|
||||||
|
|| request is BatchArchiveRequest
|
||||||
|
|| request is DeleteRequest
|
||||||
|
|| request is MoveRequest
|
||||||
|
|| request is ChangeFlagRequest
|
||||||
|
|| request is MarkReadRequest
|
||||||
|
|| request is ArchiveRequest
|
||||||
|
|| request is RenameFolderRequest
|
||||||
|
|| request is DeleteFolderRequest
|
||||||
|
|| request is AcceptEventRequest
|
||||||
|
|| request is DeclineEventRequest
|
||||||
|
|| request is OutlookDeclineEventRequest
|
||||||
|
|| request is TentativeEventRequest
|
||||||
|
|| request is UpdateCalendarEventRequest
|
||||||
|
|| request is DeleteCalendarEventRequest;
|
||||||
|
|
||||||
private static bool ShouldRevertOptimisticMailStateChange(IUIChangeRequest request)
|
private static bool ShouldRevertOptimisticMailStateChange(IUIChangeRequest request)
|
||||||
=> request is BatchMarkReadRequest
|
=> request is BatchMarkReadRequest
|
||||||
|| request is MarkReadRequest
|
|| request is MarkReadRequest
|
||||||
|
|||||||
@@ -763,7 +763,8 @@ public class ImapSynchronizer : WinoSynchronizer<ImapRequest, ImapMessageCreatio
|
|||||||
ErrorMessage = ex.Message,
|
ErrorMessage = ex.Message,
|
||||||
Exception = ex,
|
Exception = ex,
|
||||||
RequestBundle = item,
|
RequestBundle = item,
|
||||||
OperationType = "RequestExecution"
|
OperationType = "RequestExecution",
|
||||||
|
IsEntityNotFound = ex is FolderNotFoundException || ex is SynchronizerEntityNotFoundException
|
||||||
};
|
};
|
||||||
|
|
||||||
var handled = await _errorHandlerFactory.HandleErrorAsync(errorContext).ConfigureAwait(false);
|
var handled = await _errorHandlerFactory.HandleErrorAsync(errorContext).ConfigureAwait(false);
|
||||||
|
|||||||
@@ -3,6 +3,7 @@ using System.Collections.Generic;
|
|||||||
using System.Diagnostics;
|
using System.Diagnostics;
|
||||||
using System.IO;
|
using System.IO;
|
||||||
using System.Linq;
|
using System.Linq;
|
||||||
|
using System.Net;
|
||||||
using System.Net.Http;
|
using System.Net.Http;
|
||||||
using System.Text;
|
using System.Text;
|
||||||
using System.Text.Json;
|
using System.Text.Json;
|
||||||
@@ -1860,6 +1861,7 @@ public class OutlookSynchronizer : WinoSynchronizer<RequestInformation, Message,
|
|||||||
ErrorCode = (int)response.StatusCode,
|
ErrorCode = (int)response.StatusCode,
|
||||||
ErrorMessage = errorMessage,
|
ErrorMessage = errorMessage,
|
||||||
RequestBundle = bundle,
|
RequestBundle = bundle,
|
||||||
|
IsEntityNotFound = IsKnownOutlookEntityNotFoundError(response.StatusCode, errorCode, errorMessage, bundle),
|
||||||
AdditionalData = new Dictionary<string, object>
|
AdditionalData = new Dictionary<string, object>
|
||||||
{
|
{
|
||||||
{ "ErrorCode", errorCode },
|
{ "ErrorCode", errorCode },
|
||||||
@@ -1880,6 +1882,49 @@ public class OutlookSynchronizer : WinoSynchronizer<RequestInformation, Message,
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private static bool IsKnownOutlookEntityNotFoundError(
|
||||||
|
HttpStatusCode statusCode,
|
||||||
|
string errorCode,
|
||||||
|
string errorMessage,
|
||||||
|
IRequestBundle<RequestInformation> bundle)
|
||||||
|
{
|
||||||
|
if (statusCode != HttpStatusCode.NotFound || bundle?.UIChangeRequest == null)
|
||||||
|
return false;
|
||||||
|
|
||||||
|
if (!IsExistingEntityOperation(bundle.UIChangeRequest))
|
||||||
|
return false;
|
||||||
|
|
||||||
|
var normalizedErrorCode = errorCode?.Trim().ToLowerInvariant() ?? string.Empty;
|
||||||
|
var normalizedMessage = errorMessage?.Trim().ToLowerInvariant() ?? string.Empty;
|
||||||
|
|
||||||
|
return normalizedErrorCode.Contains("notfound")
|
||||||
|
|| normalizedErrorCode.Contains("itemnotfound")
|
||||||
|
|| normalizedErrorCode.Contains("resource")
|
||||||
|
|| normalizedMessage.Contains("not found")
|
||||||
|
|| normalizedMessage.Contains("does not exist")
|
||||||
|
|| normalizedMessage.Contains("cannot be found");
|
||||||
|
}
|
||||||
|
|
||||||
|
private static bool IsExistingEntityOperation(IUIChangeRequest request)
|
||||||
|
=> request is BatchDeleteRequest
|
||||||
|
|| request is BatchMoveRequest
|
||||||
|
|| request is BatchChangeFlagRequest
|
||||||
|
|| request is BatchMarkReadRequest
|
||||||
|
|| request is BatchArchiveRequest
|
||||||
|
|| request is DeleteRequest
|
||||||
|
|| request is MoveRequest
|
||||||
|
|| request is ChangeFlagRequest
|
||||||
|
|| request is MarkReadRequest
|
||||||
|
|| request is ArchiveRequest
|
||||||
|
|| request is RenameFolderRequest
|
||||||
|
|| request is DeleteFolderRequest
|
||||||
|
|| request is AcceptEventRequest
|
||||||
|
|| request is DeclineEventRequest
|
||||||
|
|| request is OutlookDeclineEventRequest
|
||||||
|
|| request is TentativeEventRequest
|
||||||
|
|| request is UpdateCalendarEventRequest
|
||||||
|
|| request is DeleteCalendarEventRequest;
|
||||||
|
|
||||||
private async Task HandleSuccessfulResponseAsync(IRequestBundle<RequestInformation> bundle, HttpResponseMessage response)
|
private async Task HandleSuccessfulResponseAsync(IRequestBundle<RequestInformation> bundle, HttpResponseMessage response)
|
||||||
{
|
{
|
||||||
try
|
try
|
||||||
|
|||||||
Reference in New Issue
Block a user