Skip to content

fix(spigot): improve diagnostics for NMS mapping drift - #73

Merged
robinbraemer merged 5 commits into
mainfrom
fm/connect-java-java26-classloader-fix
Jul 28, 2026
Merged

fix(spigot): improve diagnostics for NMS mapping drift#73
robinbraemer merged 5 commits into
mainfrom
fm/connect-java-java26-classloader-fix

Conversation

@robinbraemer

Copy link
Copy Markdown
Member

Intent

A user (quick102000) reported in a Discord support thread that the Connect Java plugin throws NoSuchMethodException and cannot be used at all on a Minecraft server running Java 26. A support bot had diagnosed this in-thread as 'Java 26 breaks Connect's reflection-based libp2p classloader isolation'. The captain publicly promised the user a fix, and the task was to diagnose, fix, and regression-test it.

I first reproduced against a real Temurin JDK 26.0.1 and DISPROVED that diagnosis. The libp2p classloader isolation works correctly and identically on JDK 26 and JDK 21: verified end-to-end by loading the actual shipped connect-spigot.jar through a simulated server plugin classloader on both JDKs (both resolve the runtime classes child-first and the reflective Libp2pTunnelTransport constructor lookup succeeds), plus every module's test suite green on 26 (core p2p 70, bedrock 71, startup 3, module 5, velocity 14, spigot 17, bungee 2). javap shows Libp2pRuntimeLoader touches only Class.getClassLoader/getProtectionDomain/URLClassLoader.getURLs, none changed in 26; jdeps shows the only JDK-internal use in the shipped jar is sun.misc.Unsafe from shaded Guava/Guice/protobuf, whose reflective surface is byte-for-byte identical on 21 and 26 (terminal-deprecation warnings only, no failure). So there is NO Java 26 classloader bug to fix.

I escalated rather than ship a fabricated fix. The likely real cause is Minecraft-version NMS drift: spigot ClassNames resolves every server internal in one static initializer whose checkNotNull calls throw, so on a brand-new Minecraft any renamed accessor takes the plugin down; a server new enough to require Java 26 is new enough to have moved its mappings, making Java 26 correlation rather than cause.

The captain then explicitly directed a bounded scope: ship ONLY the diagnosability hardening (their 'option B'). This commit is therefore deliberately diagnosability-only and does NOT attempt to fix any currently-broken server. Decisions the captain explicitly made, which should not be flagged as omissions:

  • Do NOT add speculative newest-first ClassNames accessor fallbacks. Firstmate is obtaining the user's real stack trace, server software, and MC/connect version to drive that real fix as a separate follow-up.
  • Do NOT add JDK 26 to the CI matrix. That is blocked on a Gradle 8.5 -> 9.x upgrade (pullrequest.yml already documents the JDK 21 ceiling) and was ruled explicitly out of scope.

What this change does: ClassNames failures now name the exact missing accessor plus every class name tried, the server software, the server/Bukkit/Minecraft versions, and the Java version, and state this is a Minecraft mapping change rather than a Java runtime incompatibility (the exact misdiagnosis this report attracted). Previously the operator got a bare NullPointerException(' cannot be null') on the first touch and 'NoClassDefFoundError: Could not initialize class ...ClassNames' on every touch after, neither naming server software or MC version.

Deliberate design points:

  • NmsDiagnostics is a SEPARATE class, not a member of ClassNames, specifically so it still initializes once ClassNames is in the erroneous state; that is what lets SpigotPlatform.enable() replay the original reason as a plain top-level log line rather than requiring a cause-chain walk. This separation is load-bearing, not incidental.
  • Every lookup in NmsDiagnostics is individually wrapped and degrades to 'unknown' rather than throwing, because it only ever runs while Connect is already reporting a failure and must never throw on top of it.
  • ClassNames' static initializer body was wrapped in a latching try/catch. The resulting 625-line diff is dominated by the mechanical re-indent of the body; 'git diff -w' shows the actual semantic change is small. The re-indent is intentional churn, not accidental.
  • SpigotPlatform now keeps its own ConnectLogger field because ConnectPlatform.logger is private.
  • Minecraft version is resolved via Paper's Server#getMinecraftVersion() reflectively (so it compiles and runs on plain Spigot) with a fallback to parsing Bukkit.getBukkitVersion().

Regression test (spigot NmsDiagnosticsTest, 6 tests) pins the specific gap rather than 'it loads'. Its end-to-end case loads ClassNames in a throwaway child-first classloader so its initializer runs for real and fails exactly as a drifted mapping fails on a live server, then asserts the latch still hands back the accessor-named message after the JVM has degraded to the uninformative NoClassDefFoundError. I verified this test FAILS on the pre-fix ClassNames (assertion 'the reason ClassNames failed to initialize must survive the burial ==> expected: not ') and passes on the fix. Note I corrected one of my own assertions mid-work: I had asserted the repeat NoClassDefFoundError is causeless, but the JVM does chain the original cause, so the test now asserts the accurate, still-meaningful property that its own top-level message never names the drifted accessor. Full ./gradlew build is green, and the spigot suite including the 6 new tests is also green on JDK 26.

What Changed

  • Hardened Spigot NMS lookup diagnostics with accessor, attempted class names, server and Minecraft versions, and Java runtime details.
  • Added failure-safe diagnostic latching and startup replay, plus guarded optional Velocity field access.
  • Added regression coverage and documented the mapping-drift/runtime-compatibility behavior.

Risk Assessment

✅ Low: The point-of-use guard preserves the optional plain-Spigot path, raises the named diagnostic only when Velocity support requires the missing field, and the remaining diff is bounded to the stated diagnosability hardening.

Testing

The focused NmsDiagnostics regression suite passed under Java 21 and in an actual Temurin 26.0.1 forked test JVM. Direct Java 26 output demonstrated the actionable diagnostic message. Gradle 8.5 cannot run itself on Java 26, so the test JVM was forked explicitly; generated worktree outputs were cleaned.

Evidence: Java 26 operator diagnostic

Direct Java 26 simulated-Paper diagnostic output with accessor, candidates, server/MC/Bukkit versions, Java version, and mapping-change diagnosis.

openjdk version "26.0.1" 2026-04-21
OpenJDK Runtime Environment Temurin-26.0.1+8 (build 26.0.1+8)
OpenJDK 64-Bit Server VM Temurin-26.0.1+8 (build 26.0.1+8, mixed mode, sharing)
--- Simulated Paper diagnostic emitted on Temurin 26.0.1 ---
Jul 28, 2026 2:52:56 AM org.bukkit.Bukkit setServer
INFO: This server is running Paper version git-Paper-196 (MC: 1.21.4) (Implementing API version 1.21.4-R0.1-SNAPSHOT) (Git: null on null)
java.lang.IllegalStateException: Connect could not resolve the server internal 'ServerLoginPacketListenerImpl#startClientVerification(GameProfile)' on this server. Tried: net.minecraft.server.network.ServerLoginPacketListenerImpl, net.minecraft.server.network.LoginListener. Connect binds to server internals reflectively, so this means the server's mappings differ from every name Connect knows — a Minecraft version/mapping change, not a Java runtime incompatibility. Environment: server=Paper, serverVersion=git-Paper-196 (MC: 1.21.4), bukkitVersion=1.21.4-R0.1-SNAPSHOT, minecraftVersion=1.21.4, craftbukkitPackage=jdk.proxy1, java=26.0.1 (Eclipse Adoptium). Please report this exact line at https://github.com/minekube/connect-java/issues
Evidence: Java 26 regression run

Gradle trace showing the focused tests launched with Temurin 26.0.1 and completed successfully.

Starting process 'command '/Users/robin/.local/share/mise/installs/java/temurin-26.0.1+8/bin/java''. Working directory: /Users/robin/.gradle/.tmp/tmp-jvm69430503723263193probe Command: /Users/robin/.local/share/mise/installs/java/temurin-26.0.1+8/bin/java -Xmx32m -Xms32m -cp . JavaProbe
Gradle Test Executor 1 started executing tests.
Gradle Test Executor 1 finished executing tests.
> Task :spigot:test
Starting process 'Gradle Test Executor 1'. Working directory: /Users/robin/.no-mistakes/worktrees/bd6a9ec68e4b/01KYK0TT5C562GXDDG8SRYYVV7/spigot Command: /Users/robin/.local/share/mise/installs/java/temurin-26.0.1+8/bin/java -Dorg.gradle.internal.worker.tmpdir=/Users/robin/.no-mistakes/worktrees/bd6a9ec68e4b/01KYK0TT5C562GXDDG8SRYYVV7/spigot/build/tmp/test/work -Dorg.gradle.native=false @/Users/robin/.gradle/.tmp/gradle-worker-classpath13117574742047785496txt -Xmx512m -Dfile.encoding=UTF-8 -Duser.country=US -Duser.language=en -Duser.variant -ea worker.org.gradle.process.internal.worker.GradleWorkerMain 'Gradle Test Executor 1'
Successfully started process 'Gradle Test Executor 1'
BUILD SUCCESSFUL in 33s
Evidence: NmsDiagnostics JDK 26 HTML report
<!DOCTYPE html>
<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8"/>
<meta http-equiv="x-ua-compatible" content="IE=edge"/>
<title>Test results - NmsDiagnosticsTest</title>
<link href="../css/base-style.css" rel="stylesheet" type="text/css"/>
<link href="../css/style.css" rel="stylesheet" type="text/css"/>
<script src="../js/report.js" type="text/javascript"></script>
</head>
<body>
<div id="content">
<h1>NmsDiagnosticsTest</h1>
<div class="breadcrumbs">
<a href="../index.html">all</a> &gt; 
<a href="../packages/com.minekube.connect.util.html">com.minekube.connect.util</a> &gt; NmsDiagnosticsTest</div>
<div id="summary">
<table>
<tr>
<td>
<div class="summaryGroup">
<table>
<tr>
<td>
<div class="infoBox" id="tests">
<div class="counter">7</div>
<p>tests</p>
</div>
</td>
<td>
<div class="infoBox" id="failures">
<div class="counter">0</div>
<p>failures</p>
</div>
</td>
<td>
<div class="infoBox" id="ignored">
<div class="counter">0</div>
<p>ignored</p>
</div>
</td>
<td>
<div class="infoBox" id="duration">
<div class="counter">0.183s</div>
<p>duration</p>
</div>
</td>
</tr>
</table>
</div>
</td>
<td>
<div class="infoBox success" id="successRate">
<div class="percent">100%</div>
<p>successful</p>
</div>
</td>
</tr>
</table>
</div>
<div id="tabs">
<ul class="tabLinks">
<li>
<a href="#tab0">Tests</a>
</li>
<li>
<a href="#tab1">Standard error</a>
</li>
</ul>
<div id="tab0" class="tab">
<h2>Tests</h2>
<table>
<thead>
<tr>
<th>Test</th>
<th>Duration</th>
<th>Result</th>
</tr>
</thead>
<tr>
<td class="success">classNamesInitFailureStaysReadableAfterTheCauselessNoClassDefFoundError()</td>
<td class="success">0.014s</td>
<td class="success">passed</td>
</tr>
<tr>
<td class="success">environmentDegradesInsteadOfThrowingWhenLookupsFail()</td>
<td class="success">0.001s</td>
<td class="success">passed</td>
</tr>
<tr>
<td class="success">fallbackClassFailureNamesEveryCandidateAndTheEnvironment()</td>
<td class="success">0.157s</td>
<td class="success">passed</td>
</tr>
<tr>
<td class="success">latchKeepsTheFirstFailureRatherThanALaterLessInformativeOne()</td>
<td class="success">0.002s</td>
<td class="success">passed</td>
</tr>
<tr>
<td class="success">missingAccessorNamesTheAccessorAndTheServerItWasLookedUpOn()</td>
<td class="success">0.002s</td>
<td class="success">passed</td>
</tr>
<tr>
<td class="success">missingClassListsEveryNameThatWasTried()</td>
<td class="success">0s</td>
<td class="success">passed</td>
</tr>
<tr>
<td class="success">noFailureIsReportedWhenNothingWentWrong()</td>
<td class="success">0.007s</td>
<td class="success">passed</td>
</tr>
</table>
</div>
<div id="tab1" class="tab">
<h2>Standard error</h2>
<span class="code">
<pre>Jul 28, 2026 2:46:35 AM org.bukkit.Bukkit setServer
INFO: This server is running Paper version git-Paper-196 (MC: 1.21.4) (Implementing API version 1.21.4-R0.1-SNAPSHOT) (Git: null on null)
</pre>
</span>
</div>
</div>
<div id="footer">
<p>
<div>
<label class="hidden" id="label-for-line-wrapping-toggle" for="line-wrapping-toggle">Wrap lines
<input id="line-wrapping-toggle" type="checkbox" autocomplete="off"/>
</label>
</div>Generated by 
<a href="http://www.gradle.org">Gradle 8.5</a> at Jul 28, 2026, 2:46:35 AM</p>
</div>
</div>
</body>
</html>

Pipeline

Updates from git push no-mistakes

✅ **intent** - passed

✅ No issues found.

✅ **Rebase** - passed

✅ No issues found.

🔧 **Review** - 1 issue found → auto-fixed (2) ✅
  • 🚨 spigot/src/main/java/com/minekube/connect/util/ClassNames.java:470 - Required intent says: “ClassNames failures now name the exact accessor (plus every class name that was tried), the server software, the server/Bukkit/Minecraft versions, and the Java version.” The new checkNotNull at line 470 only receives generic labels, and START_CLIENT_VERIFICATION at line 292 passes no tried-class list. Other reachable paths still bypass diagnostics entirely: getMethod can dereference a null class at line 118, while getClassOrFallback/getClassOrThrow at lines 121-124, 264-265, 307-308, and 384-387 retain raw class-not-found failures. The catch at lines 441-444 therefore can still latch an uninformative NPE/ClassNotFoundException, leaving the required diagnosability gap reachable. Normalize these lookups at the shared ClassNames lookup boundary, including all candidate names and member signatures.

🔧 Fix: Hardened ClassNames drift diagnostics across all lookup paths
1 error still open:

  • 🚨 spigot/src/main/java/com/minekube/connect/util/ClassNames.java:194 - The required fix says to “normalize EVERY ClassNames lookup ... so all drift paths produce the named, actionable diagnostic.” VELOCITY_LOGIN_MESSAGE_ID is still assigned from getField(...) without validation. If a Paper/NMS mapping renames that field, initialization succeeds with null; when Velocity support is enabled, SpigotDataHandler passes it to setValue, whose first operation dereferences it in makeAccessible, producing an unlatched NPE during login instead of an NmsDiagnostics message. Guard this lookup at the ClassNames boundary while preserving the intentionally optional non-Paper path.

🔧 Fix: Guarded optional velocity field drift at point of use
✅ Re-checked - no issues remain.

✅ **Test** - passed

✅ No issues found.

  • JAVA_HOME=/Users/robin/.local/share/mise/installs/java/temurin-21.0.11+10.0.LTS ./gradlew :spigot:test --tests com.minekube.connect.util.NmsDiagnosticsTest --no-daemon --console=plain
  • ./gradlew -I /var/folders/1y/cjgf53nj31n_dxsspqnjfjvc0000gn/T/no-mistakes-evidence/01KYK0TT5C562GXDDG8SRYYVV7/fork-tests-on-jdk26.init.gradle :spigot:test --tests com.minekube.connect.util.NmsDiagnosticsTest --no-daemon --console=plain --info --rerun-tasks
  • Temurin 26.0.1 compiled and ran the simulated-Paper NmsDiagnosticsEvidence harness
  • ./gradlew clean --no-daemon --console=plain and ./gradlew -p build-logic clean --no-daemon --console=plain
  • git status --short --branch
🔧 **Document** - 1 issue found → auto-fixed ✅
  • ⚠️ /Users/robin/.no-mistakes/worktrees/bd6a9ec68e4b/01KYK0TT5C562GXDDG8SRYYVV7/spigot/src/test/java/com/minekube/connect/util/NmsDiagnosticsTest.java:162 - Test method name still says the repeat NoClassDefFoundError is causeless; renaming it would modify test code outside this phase.

🔧 Fix: Correct diagnostic test naming
✅ Re-checked - no issues remain.

✅ **Lint** - passed

✅ No issues found.

✅ **Push** - passed

✅ No issues found.

robinbraemer and others added 5 commits July 28, 2026 02:08
…ails

ClassNames resolves every Spigot/CraftBukkit internal in one static
initializer, so a single renamed accessor on a new Minecraft release takes
the whole plugin down. What the operator got back could not be acted on: a
bare NullPointerException("<name> cannot be null") on the first touch, and
"NoClassDefFoundError: Could not initialize class ...ClassNames" on every
touch after that. Neither named the server software or the Minecraft
version, and only the first named the accessor at all.

Failures now report the exact accessor (plus every class name that was
tried), the server software, the server/Bukkit/Minecraft versions and the
Java version, and state that this is a Minecraft mapping change rather than
a Java runtime incompatibility - a misdiagnosis this class of report keeps
attracting.

NmsDiagnostics deliberately lives outside ClassNames so it still
initializes once ClassNames is in the erroneous state; that is what lets
SpigotPlatform.enable() replay the original reason as a plain top-level log
line instead of requiring a walk down a cause chain that only sometimes
carries it.

The ClassNames diff is dominated by re-indenting the initializer body into
the latching try/catch; `git diff -w` shows the actual change.

This is diagnosability only. It does not change which accessors resolve, so
it does not by itself fix any server that is currently failing - it makes
the next report say what to fix.
@robinbraemer
robinbraemer merged commit a98e723 into main Jul 28, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant