Add specific error when encoding packet with no encoder

fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/mixin/networking/UnknownCustomPayloadMixin.java

@@ -0,0 +1,58 @@
+/*
+ * Copyright (c) 2016, 2017, 2018, 2019 FabricMC
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package net.fabricmc.fabric.mixin.networking;
+
+import com.llamalad7.mixinextras.injector.ModifyReturnValue;
+import org.spongepowered.asm.mixin.Mixin;
+import org.spongepowered.asm.mixin.injection.At;
+import org.spongepowered.asm.mixin.injection.Coerce;
+
+import net.minecraft.network.PacketByteBuf;
+import net.minecraft.network.codec.PacketCodec;
+import net.minecraft.network.packet.CustomPayload;
+import net.minecraft.network.packet.UnknownCustomPayload;
+
+@Mixin(UnknownCustomPayload.class)
+public class UnknownCustomPayloadMixin {
+	/*
+	This mixin fixes an issue where a CustomPayload with no registered codec throws a confusing ClassCastException
+	when encoded
+
+	The root cause is that the vanilla encode() method has its second argument as UnknownCustomPayload
+	Even though the encode() method is empty, Java will still do an implicit cast to UnknownCustomPayload
+	This will result in a ClassCastException being thrown
+
+	This mixin creates a wrapper around the PacketCodec that uses the base interface CustomPayload as the argument to encode()
+	Without the explicit `throw new RuntimeException`, this would actually not cause any errors and encoding would succeed
+	However, it may be confusing for mod developers ("why isn't my packet being serialized?!")
+	Therefore, we opt to instead throw an explicit error stating that the payload type has no registered codec
+	*/
+	@ModifyReturnValue(method = "createCodec", at = @At("RETURN"))
+	private static <T extends PacketByteBuf> PacketCodec<T, CustomPayload> createCodec(@Coerce PacketCodec<T, CustomPayload> codec) {
+		return new PacketCodec<>() {
+			@Override
+			public CustomPayload decode(T buf) {
+				return codec.decode(buf);
+			}
+
+			@Override
+			public void encode(T buf, CustomPayload value) {
+				throw new RuntimeException("Custom payload '" + value.getId().id() + "' has no registered codec");
+			}
+		};
+	}
+}

fabric-networking-api-v1/src/main/resources/fabric-networking-api-v1.mixins.json

@@ -12,6 +12,7 @@
     "EntityTrackerEntryMixin",
     "LoginQueryRequestS2CPacketMixin",
     "LoginQueryResponseC2SPacketMixin",
+    "UnknownCustomPayloadMixin",
     "PlayerManagerMixin",
     "ServerCommonNetworkHandlerMixin",
     "ServerConfigurationNetworkHandlerMixin",

fabric-networking-api-v1/src/test/java/net/fabricmc/fabric/test/networking/unit/PayloadTypeRegistryTests.java

@@ -17,6 +17,7 @@
 package net.fabricmc.fabric.test.networking.unit;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrowsExactly;
 import static org.junit.jupiter.api.Assertions.fail;
 
 import org.junit.jupiter.api.BeforeAll;
@@ -29,6 +30,7 @@
 import net.minecraft.network.codec.PacketCodec;
 import net.minecraft.network.codec.PacketCodecs;
 import net.minecraft.network.packet.CustomPayload;
+import net.minecraft.network.packet.UnknownCustomPayload;
 import net.minecraft.network.packet.c2s.common.CustomPayloadC2SPacket;
 import net.minecraft.network.packet.s2c.common.CustomPayloadS2CPacket;
 
@@ -112,6 +114,26 @@ void S2CConfig() {
 		}
 	}
 
+	@Test
+	void handleUnregisteredCustomPayloadEncodeError() {
+		// Create packet with no registered codec
+		var packetToSend = new CustomPayloadS2CPacket(() -> CustomPayload.id("no_codec"));
+
+		// Should be *exactly* RuntimeException (with our custom message), NOT ClassCastException
+		assertThrowsExactly(RuntimeException.class, () -> {
+			CustomPayloadS2CPacket.CONFIGURATION_CODEC.encode(PacketByteBufs.create(), packetToSend);
+		});
+	}
+
+	@Test
+	void handleUnregisteredCustomPayloadDecodeAsUnknownCustomPayload() {
+		PacketByteBuf buf = PacketByteBufs.create();
+
+		buf.writeString("minecraft:no_codec");
+
+		assertEquals(UnknownCustomPayload.class, CustomPayloadS2CPacket.CONFIGURATION_CODEC.decode(buf).payload().getClass());
+	}
+
 	private record C2SPlayPayload(String value) implements CustomPayload {
 		public static final CustomPayload.Id<C2SPlayPayload> ID = CustomPayload.id("fabric:c2s_play");
 		public static final PacketCodec<RegistryByteBuf, C2SPlayPayload> CODEC = PacketCodecs.STRING.xmap(C2SPlayPayload::new, C2SPlayPayload::value).cast();

fabric-networking-api-v1/src/testmod/java/net/fabricmc/fabric/test/networking/play/NetworkingPlayPacketTest.java

@@ -79,6 +79,10 @@ public static void registerCommand(CommandDispatcher<ServerCommandSource> dispat
 					ctx.getSource().sendMessage(Text.literal("Spamming unknown packets state:" + spamUnknownPackets));
 					return Command.SINGLE_SUCCESS;
 				}))
+				.then(literal("sendMissingCodec").executes(ctx -> {
+					ServerPlayNetworking.getSender(ctx.getSource().getPlayer()).sendPacket(new CustomPayloadWithoutCodec());
+					return Command.SINGLE_SUCCESS;
+				}))
 				.then(literal("simple").executes(ctx -> {
 					ServerPlayNetworking.send(ctx.getSource().getPlayer(), new OverlayPacket(Text.literal("simple")));
 					return Command.SINGLE_SUCCESS;
@@ -123,6 +127,15 @@ public void onInitialize() {
 		});
 	}
 
+	public record CustomPayloadWithoutCodec() implements CustomPayload {
+		public static final CustomPayload.Id<CustomPayloadWithoutCodec> ID = new Id<>(NetworkingTestmods.id("no_codec"));
+
+		@Override
+		public Id<? extends CustomPayload> getId() {
+			return ID;
+		}
+	}
+
 	public record OverlayPacket(Text message) implements CustomPayload {
 		public static final CustomPayload.Id<OverlayPacket> ID = new Id<>(NetworkingTestmods.id("test_channel"));
 		public static final PacketCodec<RegistryByteBuf, OverlayPacket> CODEC = CustomPayload.codecOf(OverlayPacket::write, OverlayPacket::new);