From 55e443cd498dc1577d6da0d3d456876dab64179d Mon Sep 17 00:00:00 2001
From: William <will27528@gmail.com>
Date: Fri, 22 Sep 2023 22:07:31 +0100
Subject: [PATCH] Improve error handling on data sync

---
 .../william278/husksync/data/BukkitData.java  | 10 +----
 .../husksync/data/UserDataHolder.java         | 42 ++++++++++++++-----
 .../william278/husksync/user/OnlineUser.java  |  4 +-
 3 files changed, 37 insertions(+), 19 deletions(-)

diff --git a/bukkit/src/main/java/net/william278/husksync/data/BukkitData.java b/bukkit/src/main/java/net/william278/husksync/data/BukkitData.java
index 7c90859a..d7c17233 100644
--- a/bukkit/src/main/java/net/william278/husksync/data/BukkitData.java
+++ b/bukkit/src/main/java/net/william278/husksync/data/BukkitData.java
@@ -53,14 +53,8 @@ import java.util.stream.Collectors;
 public abstract class BukkitData implements Data {
 
     @Override
-    public final void apply(@NotNull UserDataHolder dataHolder, @NotNull HuskSync plugin) {
-        final BukkitUser user = (BukkitUser) dataHolder;
-        try {
-            this.apply(user, (BukkitHuskSync) plugin);
-        } catch (Throwable e) {
-            plugin.log(Level.WARNING, String.format("[%s] Failed to apply %s data object; skipping",
-                    user.getUsername(), this.getClass().getSimpleName()), e);
-        }
+    public final void apply(@NotNull UserDataHolder dataHolder, @NotNull HuskSync plugin) throws IllegalStateException {
+        this.apply((BukkitUser) dataHolder, (BukkitHuskSync) plugin);
     }
 
     public abstract void apply(@NotNull BukkitUser user, @NotNull BukkitHuskSync plugin) throws IllegalStateException;
diff --git a/common/src/main/java/net/william278/husksync/data/UserDataHolder.java b/common/src/main/java/net/william278/husksync/data/UserDataHolder.java
index e1a00d0f..b12c323d 100644
--- a/common/src/main/java/net/william278/husksync/data/UserDataHolder.java
+++ b/common/src/main/java/net/william278/husksync/data/UserDataHolder.java
@@ -26,6 +26,7 @@ import org.jetbrains.annotations.NotNull;
 
 import java.util.HashMap;
 import java.util.Map;
+import java.util.logging.Level;
 
 /**
  * A holder of data in the form of {@link Data}s, which can be synced
@@ -83,22 +84,40 @@ public interface UserDataHolder extends DataHolder {
      * The {@code runAfter} callback function will be run after the snapshot has been applied.
      *
      * @param snapshot the snapshot to apply
-     * @param runAfter the function to run asynchronously after the snapshot has been applied
+     * @param runAfter a consumer accepting a boolean value, indicating if the data was successfully applied,
+     *                 which will be run after the snapshot has been applied
      * @since 3.0
      */
-    default void applySnapshot(@NotNull DataSnapshot.Packed snapshot, @NotNull ThrowingConsumer<UserDataHolder> runAfter) {
+    default void applySnapshot(@NotNull DataSnapshot.Packed snapshot, @NotNull ThrowingConsumer<Boolean> runAfter) {
         final HuskSync plugin = getPlugin();
-        final DataSnapshot.Unpacked unpacked = snapshot.unpack(plugin);
+
+        // Unpack the snapshot
+        final DataSnapshot.Unpacked unpacked;
+        try {
+            unpacked = snapshot.unpack(plugin);
+        } catch (Throwable e) {
+            plugin.log(Level.SEVERE, String.format("Failed to unpack data snapshot for %s", getUsername()), e);
+            return;
+        }
+
+        // Synchronously attempt to apply the snapshot
         plugin.runSync(() -> {
-            unpacked.getData().forEach((type, data) -> {
-                if (plugin.getSettings().isSyncFeatureEnabled(type)) {
-                    if (type.isCustom()) {
-                        getCustomDataStore().put(type, data);
+            try {
+                for (Map.Entry<Identifier, Data> entry : unpacked.getData().entrySet()) {
+                    final Identifier identifier = entry.getKey();
+                    if (plugin.getSettings().isSyncFeatureEnabled(identifier)) {
+                        if (identifier.isCustom()) {
+                            getCustomDataStore().put(identifier, entry.getValue());
+                        }
+                        entry.getValue().apply(this, plugin);
                     }
-                    data.apply(this, plugin);
                 }
-            });
-            plugin.runAsync(() -> runAfter.accept(this));
+            } catch (Throwable e) {
+                plugin.log(Level.SEVERE, String.format("Failed to apply data snapshot to %s", getUsername()), e);
+                plugin.runAsync(() -> runAfter.accept(false));
+                return;
+            }
+            plugin.runAsync(() -> runAfter.accept(true));
         });
     }
 
@@ -157,6 +176,9 @@ public interface UserDataHolder extends DataHolder {
         this.setData(Identifier.PERSISTENT_DATA, persistentData);
     }
 
+    @NotNull
+    String getUsername();
+
     @NotNull
     Map<Identifier, Data> getCustomDataStore();
 
diff --git a/common/src/main/java/net/william278/husksync/user/OnlineUser.java b/common/src/main/java/net/william278/husksync/user/OnlineUser.java
index 4f28de4c..bf1e6dc0 100644
--- a/common/src/main/java/net/william278/husksync/user/OnlineUser.java
+++ b/common/src/main/java/net/william278/husksync/user/OnlineUser.java
@@ -125,12 +125,14 @@ public abstract class OnlineUser extends User implements CommandUser, UserDataHo
      * Set a player's status from a {@link DataSnapshot}
      *
      * @param snapshot The {@link DataSnapshot} to set the player's status from
+     * @param cause    The {@link DataSnapshot.UpdateCause} of the snapshot
+     * @since 3.0
      */
     public void applySnapshot(@NotNull DataSnapshot.Packed snapshot, @NotNull DataSnapshot.UpdateCause cause) {
         getPlugin().fireEvent(getPlugin().getPreSyncEvent(this, snapshot), (event) -> {
             if (!isOffline()) {
                 UserDataHolder.super.applySnapshot(
-                        event.getData(), (owner) -> completeSync(true, cause, getPlugin())
+                        event.getData(), (succeeded) -> completeSync(succeeded, cause, getPlugin())
                 );
             }
         });