Skip to content

chore(WebSocketShard): improve zlib-sync errors #10808

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

LJNeon
Copy link

@LJNeon LJNeon commented Mar 18, 2025

Please describe the changes this PR makes and why it should be merged:
The current error thrown from zlib-sync isn't great, and doesn't match what errors from node:zlib look like. This brings them more in line with node:zlib errors.

Status and versioning classification:

  • Code changes have been tested against the Discord API, or there are no code changes
  • I know how to update typings and have done so, or typings don't need updating

@LJNeon LJNeon requested a review from a team as a code owner March 18, 2025 19:35
Copy link

vercel bot commented Mar 18, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

2 Skipped Deployments
Name Status Preview Comments Updated (UTC)
discord-js ⬜️ Ignored (Inspect) Visit Preview May 28, 2025 6:58am
discord-js-guide ⬜️ Ignored (Inspect) Visit Preview May 28, 2025 6:58am

@Jiralite Jiralite added this to the ws 3.0.0 milestone May 2, 2025
@Jiralite Jiralite requested a review from almeidx May 28, 2025 18:44
@LJNeon
Copy link
Author

LJNeon commented May 28, 2025

I can't reproduce that test failure locally, not sure what happened there?

@@ -31,6 +32,17 @@ export const CompressionParameterMap = {
[CompressionMethod.ZlibSync]: 'zlib-stream',
} as const satisfies Record<CompressionMethod, string>;

export const ZlibErrorCodes = {
[ZlibSync.Z_NEED_DICT]: 'Z_NEED_DICT',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't import zlib-sync on the top level at runtime. It's being lazy loaded conditionally since it's usage is opt-in

const zLibSync = (await getZlibSync())!;

Refer to the parenthesis i included in #10808 (comment)

@github-project-automation github-project-automation bot moved this from Todo to Review in Progress in discord.js May 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Review in Progress
Development

Successfully merging this pull request may close these issues.

5 participants