Skip to content
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

Broadcast: Cannot read properties of undefined (reading 'Sender') on ws_1.WebSocket.Sender #478

Closed
offaxis opened this issue Jan 6, 2023 · 10 comments

Comments

@offaxis
Copy link

offaxis commented Jan 6, 2023

Hi !

I used to broadcast socket and worked fine ! But a new error appeared on calling client.broadcast.emit('hello');

Error:

/.../node_modules/socket.io-adapter/dist/index.js:170
            packetOpts.wsPreEncodedFrame = ws_1.WebSocket.Sender.frame(data, {
                                                          ^

TypeError: Cannot read properties of undefined (reading 'Sender')
    at RedisAdapter._encode (/.../node_modules/socket.io-adapter/dist/index.js:170:59)
    at RedisAdapter.broadcast (/.../node_modules/socket.io-adapter/dist/index.js:117:37)
    at RedisAdapter.broadcast (/.../node_modules/@socket.io/redis-adapter/dist/index.js:466:15)
    at BroadcastOperator.emit (/.../node_modules/socket.io/dist/broadcast-operator.js:169:26)
    at Socket.<anonymous> (/.../server/api/user/user.socket.js:19:34)
    at Socket.emit (node:events:513:28)
    at Socket.emit (node:domain:489:12)
    at Socket.emitUntyped (/.../node_modules/socket.io/dist/typed-events.js:69:22)
    at /.../node_modules/socket.io/dist/socket.js:614:39
    at processTicksAndRejections (node:internal/process/task_queues:77:11)

If i remove the broadcast instruction like client.emit('hello'); it works! But the current client received it too...

Versions:
"@socket.io/redis-adapter": "^8.0.0", // same issue with v7
"socket.io": "^4.5.4",
"socket.io-client": "^4.5.4",

@darrachequesne
Copy link
Member

I'm digging into this.

darrachequesne added a commit to socketio/socket.io-adapter that referenced this issue Jan 6, 2023
The named import is not supported in some cases:

> node_modules/socket.io-adapter/dist/index.js:170
>            packetOpts.wsPreEncodedFrame = ws_1.WebSocket.Sender.frame(data, {
>                                                          ^
>
> TypeError: Cannot read properties of undefined (reading 'Sender')
>     at RedisAdapter._encode (/.../node_modules/socket.io-adapter/dist/index.js:170:59)
>     at RedisAdapter.broadcast (/.../node_modules/socket.io-adapter/dist/index.js:117:37)

Related: socketio/socket.io-redis-adapter#478
@darrachequesne
Copy link
Member

This should be fixed by socketio/socket.io-adapter@99b0f18, included in socket.io-adapter@2.5.1. Could you please check by running npm update --depth 2 socket.io-adapter?

@offaxis
Copy link
Author

offaxis commented Jan 7, 2023

Thanks for the quick answer !!

It seems there is an error:

Error: Cannot find module 'ws'
Require stack:
- /usr/src/app/node_modules/socket.io-adapter/dist/index.js
- /usr/src/app/node_modules/@socket.io/redis-adapter/dist/index.js
- /usr/src/app/dist/server.bundle.js

@darrachequesne
Copy link
Member

Hmm, that's weird, ws is listed as a peer dependency, and is transitively imported by the socket.io package.

  • which version of Node.js are you using?
  • which OS?
  • did you try to remove the node_modules directory and reinstall all modules?

@offaxis
Copy link
Author

offaxis commented Jan 7, 2023

which version of Node.js are you using?
=> 18 (Docker image node:18-alpine)

which OS?
=> on local : no error but on prod server the problem appeared (AWS ubuntu/images/hvm-ssd/ubuntu-bionic-18.04-amd64-server-20200430)

did you try to remove the node_modules directory and reinstall all modules?
=> yes but same

I have listed on my project dependencies socket.io-adapter ^2.5.1 maybe it is the problem ?

@offaxis
Copy link
Author

offaxis commented Jan 9, 2023

Hi !

We found the bug: we use the command npm install --omit=dev to install project, which do not include the ws dependency witch is listed in peerDependecies in socket.io-adapter

So to fix this, i added the ws lib manually in my production dependencies.

Thanks for your work by the way!

@dszczepaniak-cksource
Copy link

Hi,

I struggle with the same issue. Is the way described in #478 (comment) the recommended way to resolve the issue? cc @darrachequesne

@darrachequesne
Copy link
Member

Unfortunately, I'm not able to reproduce the issue:

$ node -v
v18.12.1

$ npm install --omit=dev socket.io @socket.io/redis-adapter redis

added 26 packages, and audited 27 packages in 742ms

found 0 vulnerabilities

$ npm ls ws
tmp@ /tmp
├─┬ @socket.io/redis-adapter@8.0.0
│ └─┬ socket.io-adapter@2.5.1
│   └── ws@8.12.0
└─┬ socket.io@4.5.4
  └─┬ engine.io@6.2.1
    └── ws@8.2.3

I guess we can list ws in dependencies instead of peerDependencies, in order to prevent this issue, but I don't understand why it's not working as expected.

Related: #481

@offaxis
Copy link
Author

offaxis commented Jan 10, 2023

We use npm v8, it seems to be important.

The manual installation of ws is a patch, to unblock the situation, but we hope, it will be added as dependencies in socket.io-adapter

darrachequesne added a commit that referenced this issue Jan 11, 2023
- `socket.io@latest` depends on `socket.io-adapter@~2.4.0`, which resolves to `socket.io-adapter@2.4.0`
- `@socket.io/redis-adapter@latest` depends on `socket.io-adapter@^2.4.0`, which resolves to `socket.io-adapter@2.5.1` (released a few days ago)

Typescript complains that the two types do not match:

> Argument of type '(nsp: any) => RedisAdapter' is not assignable to parameter of type 'AdapterConstructor'.
>  Type '(nsp: any) => RedisAdapter' is not assignable to type '(nsp: Namespace<DefaultEventsMap, DefaultEventsMap, DefaultEventsMap, any>) => Adapter'.
>    Type 'RedisAdapter' is not assignable to type 'Adapter'.
>      Types have separate declarations of a private property 'encoder'.

Related:

- #478
- #482
@darrachequesne
Copy link
Member

This should be fixed by 7aecf01, included in version 8.0.1.

darrachequesne added a commit to socketio/socket.io-adapter that referenced this issue Jan 12, 2023
In order to prevent issues like [1].

Note: the version matches the one imported by the `engine.io` package

See: https://github.com/socketio/engine.io/blob/6.3.0/package.json#L43

[1]: socketio/socket.io-redis-adapter#478
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

No branches or pull requests

3 participants