Backport to 1.21.1: Avoid swallowing errors in case of AttachmentType…

… Serialization failure (#1955) Co-authored-by: TheSilkMiner <[email protected]>
neoforged · Feb 12, 2025 · 54b964c · 54b964c
1 parent 1acab57
commit 54b964c
Show file tree

Hide file tree

Showing 2 changed files with 22 additions and 8 deletions.
diff --git a/src/main/java/net/neoforged/neoforge/attachment/AttachmentHolder.java b/src/main/java/net/neoforged/neoforge/attachment/AttachmentHolder.java
@@ -121,12 +121,17 @@ public final CompoundTag serializeAttachments(HolderLookup.Provider provider) {
         CompoundTag tag = null;
         for (var entry : attachments.entrySet()) {
             var type = entry.getKey();
+            var key = NeoForgeRegistries.ATTACHMENT_TYPES.getKey(type);
             if (type.serializer != null) {
-                Tag serialized = ((IAttachmentSerializer<?, Object>) type.serializer).write(entry.getValue(), provider);
-                if (serialized != null) {
-                    if (tag == null)
-                        tag = new CompoundTag();
-                    tag.put(NeoForgeRegistries.ATTACHMENT_TYPES.getKey(type).toString(), serialized);
+                try {
+                    Tag serialized = ((IAttachmentSerializer<?, Object>) type.serializer).write(entry.getValue(), provider);
+                    if (serialized != null) {
+                        if (tag == null)
+                            tag = new CompoundTag();
+                        tag.put(key.toString(), serialized);
+                    }
+                } catch (Exception exception) {
+                    LOGGER.error("Failed to serialize data attachment {}. Skipping.", key, exception);
                 }
             }
         }

diff --git a/src/main/java/net/neoforged/neoforge/attachment/AttachmentType.java b/src/main/java/net/neoforged/neoforge/attachment/AttachmentType.java
@@ -7,6 +7,7 @@
 
 import com.google.common.base.Predicates;
 import com.mojang.serialization.Codec;
+import com.mojang.serialization.DataResult;
 import java.util.Objects;
 import java.util.function.Function;
 import java.util.function.Predicate;
@@ -198,17 +199,25 @@ public Builder<T> serialize(Codec<T> codec) {
          */
         public Builder<T> serialize(Codec<T> codec, Predicate<? super T> shouldSerialize) {
             Objects.requireNonNull(codec);
-            // TODO: better error handling
             return serialize(new IAttachmentSerializer<>() {
                 @Override
                 public T read(IAttachmentHolder holder, Tag tag, HolderLookup.Provider provider) {
-                    return codec.parse(provider.createSerializationContext(NbtOps.INSTANCE), tag).result().get();
+                    final DataResult<T> parsingResult = codec.parse(provider.createSerializationContext(NbtOps.INSTANCE), tag);
+                    return parsingResult.getOrThrow(msg -> buildException("read", msg));
                 }
 
                 @Nullable
                 @Override
                 public Tag write(T attachment, HolderLookup.Provider provider) {
-                    return shouldSerialize.test(attachment) ? codec.encodeStart(provider.createSerializationContext(NbtOps.INSTANCE), attachment).result().get() : null;
+                    if (!shouldSerialize.test(attachment)) {
+                        return null;
+                    }
+                    final DataResult<Tag> encodingResult = codec.encodeStart(provider.createSerializationContext(NbtOps.INSTANCE), attachment);
+                    return encodingResult.getOrThrow(msg -> buildException("write", msg));
+                }
+
+                private RuntimeException buildException(final String operation, final String error) {
+                    return new IllegalStateException("Unable to " + operation + " attachment due to an internal codec error: " + error);
                 }
             });
         }