Improve exception handling

This commit is contained in:
Nassim Jahnke 2023-01-06 20:33:17 +01:00
parent 8d7270a2ff
commit 4067107b52
No known key found for this signature in database
GPG Key ID: 6BE3B555EBC5982B
6 changed files with 72 additions and 23 deletions

View File

@ -334,8 +334,9 @@ public abstract class AbstractProtocol<C1 extends ClientboundPacketType, C2 exte
} }
private void throwRemapError(Direction direction, State state, int oldId, int newId, InformativeException e) throws InformativeException { private void throwRemapError(Direction direction, State state, int oldId, int newId, InformativeException e) throws InformativeException {
// Don't print errors during handshake // Don't print errors during handshake/login/status
if (state == State.HANDSHAKE) { if (state != State.PLAY && direction == Direction.SERVERBOUND && !Via.getManager().debugHandler().enabled()) {
e.setShouldBePrinted(false);
throw e; throw e;
} }
@ -369,6 +370,7 @@ public abstract class AbstractProtocol<C1 extends ClientboundPacketType, C2 exte
@Override @Override
public @Nullable <T> T get(Class<T> objectClass) { public @Nullable <T> T get(Class<T> objectClass) {
//noinspection unchecked
return (T) storedObjects.get(objectClass); return (T) storedObjects.get(objectClass);
} }

View File

@ -27,6 +27,7 @@ import java.util.Map;
public class InformativeException extends Exception { public class InformativeException extends Exception {
private final Map<String, Object> info = new HashMap<>(); private final Map<String, Object> info = new HashMap<>();
private boolean shouldBePrinted = true;
private int sources; private int sources;
public InformativeException(Throwable cause) { public InformativeException(Throwable cause) {
@ -46,6 +47,14 @@ public class InformativeException extends Exception {
return sourceClazz.isAnonymousClass() ? sourceClazz.getName() + " (Anonymous)" : sourceClazz.getName(); return sourceClazz.isAnonymousClass() ? sourceClazz.getName() + " (Anonymous)" : sourceClazz.getName();
} }
public boolean shouldBePrinted() {
return shouldBePrinted;
}
public void setShouldBePrinted(final boolean shouldBePrinted) {
this.shouldBePrinted = shouldBePrinted;
}
@Override @Override
public String getMessage() { public String getMessage() {
StringBuilder builder = new StringBuilder("Please report this on the Via support Discord or open an issue on the relevant GitHub repository\n"); StringBuilder builder = new StringBuilder("Please report this on the Via support Discord or open an issue on the relevant GitHub repository\n");

View File

@ -28,6 +28,7 @@ import io.netty.channel.ChannelPipeline;
import io.netty.handler.codec.ByteToMessageDecoder; import io.netty.handler.codec.ByteToMessageDecoder;
import io.netty.handler.codec.MessageToByteEncoder; import io.netty.handler.codec.MessageToByteEncoder;
import io.netty.handler.codec.MessageToMessageDecoder; import io.netty.handler.codec.MessageToMessageDecoder;
import org.checkerframework.checker.nullness.qual.Nullable;
import java.lang.reflect.InvocationTargetException; import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method; import java.lang.reflect.Method;
@ -116,6 +117,25 @@ public final class PipelineUtil {
return false; return false;
} }
/**
* Check if a stack trace contains a certain exception and returns it if present.
*
* @param t throwable
* @param c the exception to look for
* @return contained exception of t if present
*/
public static <T> @Nullable T getCause(Throwable t, Class<T> c) {
while (t != null) {
if (c.isAssignableFrom(t.getClass())) {
//noinspection unchecked
return (T) t;
}
t = t.getCause();
}
return null;
}
/** /**
* Get the context for a the channel handler before a certain name. * Get the context for a the channel handler before a certain name.
* *

View File

@ -17,9 +17,7 @@
*/ */
package com.viaversion.viaversion.bukkit.handlers; package com.viaversion.viaversion.bukkit.handlers;
import com.viaversion.viaversion.api.Via;
import com.viaversion.viaversion.api.connection.UserConnection; import com.viaversion.viaversion.api.connection.UserConnection;
import com.viaversion.viaversion.api.protocol.packet.State;
import com.viaversion.viaversion.bukkit.util.NMSUtil; import com.viaversion.viaversion.bukkit.util.NMSUtil;
import com.viaversion.viaversion.exception.CancelCodecException; import com.viaversion.viaversion.exception.CancelCodecException;
import com.viaversion.viaversion.exception.CancelDecoderException; import com.viaversion.viaversion.exception.CancelDecoderException;
@ -67,9 +65,15 @@ public final class BukkitDecodeHandler extends MessageToMessageDecoder<ByteBuf>
} }
super.exceptionCaught(ctx, cause); super.exceptionCaught(ctx, cause);
if (!NMSUtil.isDebugPropertySet() && PipelineUtil.containsCause(cause, InformativeException.class) if (NMSUtil.isDebugPropertySet()) {
&& (connection.getProtocolInfo().getState() != State.HANDSHAKE || Via.getManager().isDebug())) { return;
cause.printStackTrace(); // Print if CB doesn't already do it }
// Print if CB doesn't already do it
final InformativeException exception = PipelineUtil.getCause(cause, InformativeException.class);
if (exception != null && exception.shouldBePrinted()) {
cause.printStackTrace();
exception.setShouldBePrinted(false);
} }
} }

View File

@ -17,9 +17,7 @@
*/ */
package com.viaversion.viaversion.bukkit.handlers; package com.viaversion.viaversion.bukkit.handlers;
import com.viaversion.viaversion.api.Via;
import com.viaversion.viaversion.api.connection.UserConnection; import com.viaversion.viaversion.api.connection.UserConnection;
import com.viaversion.viaversion.api.protocol.packet.State;
import com.viaversion.viaversion.bukkit.util.NMSUtil; import com.viaversion.viaversion.bukkit.util.NMSUtil;
import com.viaversion.viaversion.exception.CancelCodecException; import com.viaversion.viaversion.exception.CancelCodecException;
import com.viaversion.viaversion.exception.CancelEncoderException; import com.viaversion.viaversion.exception.CancelEncoderException;
@ -110,9 +108,15 @@ public final class BukkitEncodeHandler extends MessageToMessageEncoder<ByteBuf>
} }
super.exceptionCaught(ctx, cause); super.exceptionCaught(ctx, cause);
if (!NMSUtil.isDebugPropertySet() && PipelineUtil.containsCause(cause, InformativeException.class) if (NMSUtil.isDebugPropertySet()) {
&& (connection.getProtocolInfo().getState() != State.HANDSHAKE || Via.getManager().isDebug())) { return;
cause.printStackTrace(); // Print if CB doesn't already do it }
// Print if CB doesn't already do it
final InformativeException exception = PipelineUtil.getCause(cause, InformativeException.class);
if (exception != null && exception.shouldBePrinted()) {
cause.printStackTrace();
exception.setShouldBePrinted(false);
} }
} }

View File

@ -49,7 +49,9 @@ public class PacketWrapperImpl implements PacketWrapper {
private final ByteBuf inputBuffer; private final ByteBuf inputBuffer;
private final UserConnection userConnection; private final UserConnection userConnection;
private boolean send = true; private boolean send = true;
/** Only non-null if specifically set and gotten before packet transformation */ /**
* Only non-null if specifically set and gotten before packet transformation
*/
private PacketType packetType; private PacketType packetType;
private int id; private int id;
private final Deque<Pair<Type, Object>> readableObjects = new ArrayDeque<>(); private final Deque<Pair<Type, Object>> readableObjects = new ArrayDeque<>();
@ -78,9 +80,7 @@ public class PacketWrapperImpl implements PacketWrapper {
} }
currentIndex++; currentIndex++;
} }
throw createInformativeException(new ArrayIndexOutOfBoundsException("Could not find type " + type.getTypeName() + " at " + index), type, index);
Exception e = new ArrayIndexOutOfBoundsException("Could not find type " + type.getTypeName() + " at " + index);
throw new InformativeException(e).set("Type", type.getTypeName()).set("Index", index).set("Packet ID", getId()).set("Packet Type", packetType).set("Data", packetValues);
} }
@Override @Override
@ -121,20 +121,22 @@ public class PacketWrapperImpl implements PacketWrapper {
} }
currentIndex++; currentIndex++;
} }
Exception e = new ArrayIndexOutOfBoundsException("Could not find type " + type.getTypeName() + " at " + index); throw createInformativeException(new ArrayIndexOutOfBoundsException("Could not find type " + type.getTypeName() + " at " + index), type, index);
throw new InformativeException(e).set("Type", type.getTypeName()).set("Index", index).set("Packet ID", getId()).set("Packet Type", packetType);
} }
@Override @Override
public <T> T read(Type<T> type) throws Exception { public <T> T read(Type<T> type) throws Exception {
if (type == Type.NOTHING) return null; if (type == Type.NOTHING) {
return null;
}
if (readableObjects.isEmpty()) { if (readableObjects.isEmpty()) {
Preconditions.checkNotNull(inputBuffer, "This packet does not have an input buffer."); Preconditions.checkNotNull(inputBuffer, "This packet does not have an input buffer.");
// We could in the future log input read values, but honestly for things like bulk maps, mem waste D: // We could in the future log input read values, but honestly for things like bulk maps, mem waste D:
try { try {
return type.read(inputBuffer); return type.read(inputBuffer);
} catch (Exception e) { } catch (Exception e) {
throw new InformativeException(e).set("Type", type.getTypeName()).set("Packet ID", getId()).set("Packet Type", packetType).set("Data", packetValues); throw createInformativeException(e, type, packetValues.size() + 1);
} }
} }
@ -147,8 +149,7 @@ public class PacketWrapperImpl implements PacketWrapper {
} else if (rtype == Type.NOTHING) { } else if (rtype == Type.NOTHING) {
return read(type); // retry return read(type); // retry
} else { } else {
Exception e = new IOException("Unable to read type " + type.getTypeName() + ", found " + read.key().getTypeName()); throw createInformativeException(new IOException("Unable to read type " + type.getTypeName() + ", found " + read.key().getTypeName()), type, readableObjects.size());
throw new InformativeException(e).set("Type", type.getTypeName()).set("Packet ID", getId()).set("Packet Type", packetType).set("Data", packetValues);
} }
} }
@ -209,13 +210,22 @@ public class PacketWrapperImpl implements PacketWrapper {
try { try {
packetValue.key().write(buffer, packetValue.value()); packetValue.key().write(buffer, packetValue.value());
} catch (Exception e) { } catch (Exception e) {
throw new InformativeException(e).set("Index", index).set("Type", packetValue.key().getTypeName()).set("Packet ID", getId()).set("Packet Type", packetType).set("Data", packetValues); throw createInformativeException(e, packetValue.key(), index);
} }
index++; index++;
} }
writeRemaining(buffer); writeRemaining(buffer);
} }
private InformativeException createInformativeException(final Exception cause, final Type<?> type, final int index) {
return new InformativeException(cause)
.set("Index", index)
.set("Type", type.getTypeName())
.set("Packet ID", this.id)
.set("Packet Type", this.packetType)
.set("Data", this.packetValues);
}
@Override @Override
public void clearInputBuffer() { public void clearInputBuffer() {
if (inputBuffer != null) { if (inputBuffer != null) {