Surface clock drift errors with actionable messages (#6789)

* Initial plan

* Add clock-drift error handling to sync flow

Co-authored-by: MatissJanis <886567+MatissJanis@users.noreply.github.com>

* Add release notes for PR #6789

* Refactor error handling in sync flow

- Extracted error handling logic into a separate function for better readability and maintainability.
- Updated the receiveMessages and _sendMessages functions to utilize the new errorHandler.
- Removed redundant error handling code from the _sendMessages function.
- Enhanced error handling for SyncError and ClockDriftError to emit specific events.

* Enhance error handling in receiveMessages function

- Added try-catch block to handle Timestamp.ClockDriftError specifically.
- Emitted a sync error event for clock-drift errors to improve synchronization feedback.
- Maintained existing functionality while improving error reporting.

* Update authorship in release notes and refine clock-drift error messaging for clarity

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: MatissJanis <886567+MatissJanis@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Matiss Janis Aboltins <matiss@mja.lv>

packages/desktop-client/src/sync-events.ts

@@ -344,6 +344,16 @@ export function listenForSyncEvent(store: AppStore) {
         case 'network':
           // Show nothing
           break;
+        case 'clock-drift':
+          notif = {
+            title: t('Time sync issue'),
+            message: t(
+              'Failed to sync because your device time differs too much from the server. Please check your device time settings and ensure they are correct.',
+            ),
+            type: 'warning',
+            sticky: true,
+          };
+          break;
         case 'token-expired':
           notif = {
             title: 'Login expired',

packages/loot-core/src/server/sync/index.ts

@@ -421,33 +421,51 @@ export const applyMessages = sequential(async (messages: Message[]) => {
 });
 
 export function receiveMessages(messages: Message[]): Promise<Message[]> {
-  messages.forEach(msg => {
-    Timestamp.recv(msg.timestamp);
-  });
+  try {
+    messages.forEach(msg => {
+      Timestamp.recv(msg.timestamp);
+    });
+  } catch (e) {
+    if (e instanceof Timestamp.ClockDriftError) {
+      throw new SyncError('clock-drift');
+    }
+    throw e;
+  }
 
   return runMutator(() => applyMessages(messages));
 }
 
+async function errorHandler(e: Error) {
+  captureException(e);
+
+  if (e instanceof SyncError) {
+    if (e.reason === 'invalid-schema') {
+      // We know this message came from a local modification, and it
+      // couldn't apply, which doesn't make any sense. Must be a bug
+      // in the code. Send a specific error type for it for a custom
+      // message.
+      app.events.emit('sync', {
+        type: 'error',
+        subtype: 'apply-failure',
+        meta: e.meta,
+      });
+    } else {
+      app.events.emit('sync', { type: 'error', meta: e.meta });
+    }
+  } else if (e instanceof Timestamp.ClockDriftError) {
+    app.events.emit('sync', {
+      type: 'error',
+      subtype: 'clock-drift',
+      meta: { message: e.message },
+    });
+  }
+}
+
 async function _sendMessages(messages: Message[]): Promise<void> {
   try {
     await applyMessages(messages);
   } catch (e) {
-    if (e instanceof SyncError) {
-      if (e.reason === 'invalid-schema') {
-        // We know this message came from a local modification, and it
-        // couldn't apply, which doesn't make any sense. Must be a bug
-        // in the code. Send a specific error type for it for a custom
-        // message.
-        app.events.emit('sync', {
-          type: 'error',
-          subtype: 'apply-failure',
-          meta: e.meta,
-        });
-      } else {
-        app.events.emit('sync', { type: 'error', meta: e.meta });
-      }
-    }
-
+    errorHandler(e);
     throw e;
   }
 
@@ -467,6 +485,8 @@ export async function batchMessages(func: () => Promise<void>): Promise<void> {
 
   try {
     await func();
+  } catch (e) {
+    errorHandler(e);
     // TODO: if it fails, it shouldn't apply them?
   } finally {
     IS_BATCHING = false;
@@ -586,6 +606,12 @@ export const fullSync = once(async function (): Promise<
           subtype: e.reason,
           meta: e.meta,
         });
+      } else if (e.reason === 'clock-drift') {
+        app.events.emit('sync', {
+          type: 'error',
+          subtype: 'clock-drift',
+          meta: e.meta,
+        });
       } else {
         app.events.emit('sync', { type: 'error', meta: e.meta });
       }

packages/loot-core/src/shared/errors.ts

@@ -72,6 +72,11 @@ export function getDownloadError({
         'This budget cannot be loaded with this version of the app. Make sure the app is up-to-date.',
       );
 
+    case 'clock-drift':
+      return t(
+        'Failed to download the budget because your device time differs too much from the server. Please check your device time settings and ensure they are correct.',
+      );
+
     default:
       const info =
         meta && typeof meta === 'object' && 'fileId' in meta && meta.fileId
@@ -115,6 +120,10 @@ export function getSyncError(error, id) {
       'Budget "{{id}}" not found. Check the ID of your budget in the Advanced section of the settings page.',
       { id },
     );
+  } else if (error === 'clock-drift') {
+    return t(
+      'Failed to sync because your device time differs too much from the server. Please check your device time settings and ensure they are correct.',
+    );
   } else {
     return t('We had an unknown problem opening "{{id}}".', { id });
   }

upcoming-release-notes/6789.md

@@ -0,0 +1,6 @@
+---
+category: Enhancements
+authors: [MatissJanis]
+---
+
+Add user-friendly error message for clock-drift issues