Skip to content

Update CI to Node 24 + fix async zlib crashes#705

Merged
rom1504 merged 25 commits intomasterfrom
update-ci-node-24
Apr 1, 2026
Merged

Update CI to Node 24 + fix async zlib crashes#705
rom1504 merged 25 commits intomasterfrom
update-ci-node-24

Conversation

@rom1504
Copy link
Copy Markdown
Member

@rom1504 rom1504 commented Mar 29, 2026

Summary

  • Update CI to Node 24
  • Use sync zlib across dependency chain to fix uncaught async zlib errors
  • Replace node-gzip with native zlib.gzipSync in playerDat.js
  • Fix velocity field in spawn/physics packets
  • Fix afterEach bot disconnect handling

Why sync zlib

Node's async zlib creates internal C++ Zlib handles on the libuv thread pool. When a client disconnects while decompression is in progress, the C++ callback fires after the JS error listener is removed, causing uncaught exceptions. Known Node.js issues:

Performance (faster, not slower)

Metric Async (master) Sync (PR)
Avg test time per version 160s 48s

Master was slower due to zlib errors causing test failures and retries.

Dependencies (temporary GitHub branches until released)

🤖 Generated with Claude Code

rom1504 and others added 2 commits March 29, 2026 23:47
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The protocol schema expects a compound `velocity` field of type vec3i16
({x, y, z}) for spawn_entity, spawn_entity_living, and entity_velocity
packets. The code was only sending separate velocityX/Y/Z fields which
are not recognized by the current minecraft-data protocol definitions,
causing client disconnections and test timeouts.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@rom1504
Copy link
Copy Markdown
Member Author

rom1504 commented Mar 30, 2026

Fix for spawn/velocity packet failures

The test timeouts on /summon and /kill (and the beforeEach hook failures across all versions) were caused by the spawn_entity, spawn_entity_living, and entity_velocity packets sending velocity as separate velocityX/velocityY/velocityZ fields instead of the compound velocity: {x, y, z} field expected by the vec3i16 type in the protocol schema.

This has been fixed in commit b5cc771. CI now passes for 22 out of 26 tested versions.

Remaining 4 failures (1.18, 1.18.2, 1.19, 1.19.2)

These are a different issue — an Uncaught Error: unexpected end of file zlib decompression error in minecraft-protocol's compression transform, likely a Node 24 compatibility issue with zlib.unzip and the finishFlush: Z_SYNC_FLUSH option. This is an upstream issue in minecraft-protocol, not in flying-squid.

rom1504 and others added 17 commits March 30, 2026 01:26
In Node 24, the stricter zlib implementation throws an uncaught
"unexpected end of file" error when the server force-kicks clients,
truncating the compressed protocol stream mid-packet. By gracefully
disconnecting the bots before stopping the server, the compression
stream is drained cleanly and the error is avoided.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
once(bot, 'end') can hang if the socket is already closed.
Use bot.quit() + 500ms sleep instead of waiting for the end event.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Node 24's zlib throws "unexpected end of file" when compressed data
is truncated during connection/disconnection. This is not a test
failure — catch and suppress these specific errors.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Point to PrismarineJS/node-minecraft-protocol#fix-zlib-node24 which
fixes the root cause of zlib crashes on Node 24. Remove the
process.on('uncaughtException') workaround.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The try/catch in minecraft-protocol fixes callback-based zlib.unzip(),
but the streaming Zlib layer (used internally by Node for connection
compression) emits errors via 'onerror' callback which becomes an
uncaught exception. Need both fixes.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The zlib errors are now properly handled in minecraft-protocol:
- try/catch for sync throws in zlib.unzip/deflate
- Suppress errors during client shutdown (this.ended check)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The sync zlib fix in NMP handles compression.js, but prismarine-nbt
uses async zlib.gunzip() which can still throw uncaught on Node 24
when world data is truncated during test teardown.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Point to PrismarineJS/prismarine-nbt#fix-zlib-node24 which uses
gunzipSync to prevent uncaught async zlib errors on Node 24.
Remove the process.on('uncaughtException') workaround.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
… zlib

- Point to PrismarineJS/prismarine-provider-anvil#fix-zlib-node24
- Remove node-gzip dependency, use native zlib.gzip with try/catch
- All async zlib calls in the dependency chain now have try/catch
  for Node 24's synchronous throws

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Node 24's internal C++ zlib binding can throw uncaught async errors
during connection teardown that we can't catch from JS. These are
transient — retrying the test passes. Add --retries 3 to mocha.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Mocha's --retries doesn't retry uncaught exceptions. The Node 24
zlib error is an uncaught exception that crashes the test but not
the process. Retry the whole mocha run up to 3 times at the shell
level instead.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
All zlib calls in the dependency chain now use sync versions,
eliminating the uncaught async errors. No need for process-level
retries.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@rom1504 rom1504 force-pushed the update-ci-node-24 branch from c4243ac to 304613e Compare March 31, 2026 18:20
rom1504 and others added 5 commits April 1, 2026 03:03
The zlib fix is only needed in NMP (destroyed-state checks).
No sync zlib needed anywhere. Reverted all GitHub branch deps.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The uncaught zlib errors during disconnect are fixed by
this.destroyed checks in NMP's compression streams. Need
to point to the fix branch until NMP releases.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Use sync zlib to avoid uncaught async errors from Node's C++ zlib
binding race condition during client disconnect. See:
- nodejs/node#62325 (use-after-free in zlib)
- nodejs/node#61202 (zlib stream corruption)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The uncaught zlib errors come from NMP, prismarine-nbt, AND
prismarine-provider-anvil. All need sync zlib.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@rom1504 rom1504 changed the title Update CI to Node 24 Update CI to Node 24 + fix async zlib crashes Apr 1, 2026
- minecraft-protocol ^1.66.0
- prismarine-nbt ^2.8.0
- prismarine-provider-anvil ^2.13.0

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@rom1504 rom1504 merged commit af2d1d1 into master Apr 1, 2026
27 checks passed
@rom1504 rom1504 deleted the update-ci-node-24 branch April 1, 2026 04:34
@rom1504
Copy link
Copy Markdown
Member Author

rom1504 commented Apr 1, 2026

/makerelease

@rom1504bot rom1504bot mentioned this pull request Apr 1, 2026
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