THRIFT-6039: Upgrade ws from 5.2.x to 8.21.0#3526
Open
jimexist wants to merge 2 commits into
Open
Conversation
Client: nodejs The ws package was pinned at ^5.2.3 (resolved 5.2.4), which is several major versions behind the current 8.21.0 release. ws is consumed indirectly via isomorphic-ws in lib/nodejs/lib/thrift/ws_connection.js; only the client-side `new WebSocket(uri, "", wsOptions)` constructor is used, which is unchanged across the v5 -> v8 transition. No server-side ws.Server API is used in the codebase. ws v8 requires Node.js >= 10.0.0; package.json's engines.node is already >= 10.18.0, so the engine constraint is satisfied. The transitive async-limiter dependency (only needed by ws 5.x) is removed from the lock file. ws is BSD-2-Clause licensed (ASF Category A), so no LICENSE or NOTICE updates are required. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Generated-by: Claude Opus 4.7 (1M context)
There was a problem hiding this comment.
Pull request overview
Upgrades the Node.js binding’s WebSocket dependency (ws) to a supported major version, aligning the dependency set with the project’s existing Node engine constraints and removing legacy transitive packages from the lockfile.
Changes:
- Bump
wsfrom^5.2.3to^8.21.0inpackage.json. - Refresh
package-lock.jsonto resolvews@8.21.0and dropasync-limiter, updating related lock metadata.
Reviewed changes
Copilot reviewed 1 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| package.json | Updates the direct ws dependency to ^8.21.0. |
| package-lock.json | Resolves ws@8.21.0, removes async-limiter, and updates dependency metadata accordingly. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
jimexist
changed the title
Client: nodejs ws v8 added strict validation of the protocols argument and rejects an empty string with `SyntaxError: An invalid or duplicated subprotocol was specified`. ws v5 tolerated `""` silently. WSConnection passed `""` as the protocols argument when running on Node (browser code path is unaffected). Switch to `undefined` so the constructor receives no subprotocol, matching both the WHATWG WebSocket and ws documented signatures. Surfaced by the lib-nodejs CI job after the ws 5 -> 8 upgrade. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Generated-by: Claude Opus 4.7 (1M context)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Upgrades the
wsWebSocket library from^5.2.3(resolved 5.2.4) to^8.21.0. ws 5.x has been EOL for years and is several major versions behind the current release.wsis consumed indirectly throughisomorphic-wsin lib/nodejs/lib/thrift/ws_connection.js; only the client-sidenew WebSocket(uri, "", wsOptions)constructor is used, which is unchanged across v5 → v8. No server-sidews.ServerAPI is used in the codebase, so the breaking changes between v5 and v8 (most of which affect the server API) do not apply.Compatibility
package.json'sengines.nodeis already>= 10.18.0, so the constraint is satisfied.async-limiterdependency (only needed by ws 5.x) is removed.wsis BSD-2-Clause (ASF Category A), so noLICENSE/NOTICEupdates are required.Notes for reviewers
AGENTS.md/CONTRIBUTING.md(Co-Authored-By/Generated-by).Test plan
npm installresolves cleanlylib/nodejs/test/testAll.shpasseslib/nodejs/test/server.js+ WS client) succeeds🤖 Generated with Claude Code