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.

Sign up for GitHub

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

Jump to bottom

streammuxer order changes behaviour #246

Closed
vmx opened this issue Sep 13, 2018 · 1 comment · Fixed by #1779
Closed

streammuxer order changes behaviour #246

vmx opened this issue Sep 13, 2018 · 1 comment · Fixed by #1779
Labels
exp/novice Someone with a little familiarity can pick up good first issue Good issue for new contributors help wanted Seeking public contribution on this issue topic/docs Documentation

Comments

@vmx
Copy link
Contributor

vmx commented Sep 13, 2018

  • Version: 0.23.1
  • Platform: Linux

Type: Bug

Severity: High

Description:

When sending data > 8k to a client it gets chunked into 8k blocks when the streamMuxer is set to [ spdy, mplex ]. If it is set to [ mplex, spdy ] it works as expected and the client receives the full block.

So the order of the stream muxers matter.

Steps to reproduce the error:

Checkout the repo at https://github.com/vmx/libp2p-streammuxer-bug and follow the instructions in the README.
@daviddias daviddias added status/ready Ready to be worked kind/bug A bug in existing code (including security flaws) labels Sep 24, 2018
@achingbrain
Copy link
Member

Muxers are negotiated during multistream select. spdy is no longer used but we do now have yamux in addition to mplex, plus some transports like WebTransport and WebRTC come with their own muxers.

I think this needs a documentation update to say that the order that muxers are configured in affects which muxer is chosen (typically the first in the list).

A section should be added to Stream Multiplexing in the configuration docs.

@achingbrain achingbrain added help wanted Seeking public contribution on this issue exp/novice Someone with a little familiarity can pick up good first issue Good issue for new contributors topic/docs Documentation and removed kind/bug A bug in existing code (including security flaws) status/ready Ready to be worked labels Oct 5, 2022
@p-shahi p-shahi mentioned this issue May 30, 2023
Merged
maschad pushed a commit to maschad/js-libp2p that referenced this issue Jun 21, 2023
@dependabot
deps(dev): bump @libp2p/interface-mocks from 8.0.5 to 9.1.1 (libp2p#246)
6f01bd1 
Bumps [@libp2p/interface-mocks](https://github.com/libp2p/js-libp2p-interfaces) from 8.0.5 to 9.1.1.
- [Release notes](https://github.com/libp2p/js-libp2p-interfaces/releases)
- [Commits](https://github.com/libp2p/js-libp2p-interfaces/compare/@libp2p/interface-mocks-v8.0.5...@libp2p/interface-mocks-v9.1.1)

---
updated-dependencies:
- dependency-name: "@libp2p/interface-mocks"
  dependency-type: direct:development
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
maschad pushed a commit to maschad/js-libp2p that referenced this issue Jun 21, 2023
@semantic-release-bot
chore(release): 6.1.2 [skip ci]
6ea251e 
## [6.1.2](libp2p/js-libp2p-tcp@v6.1.1...v6.1.2) (2023-02-01)

### Dependencies

* **dev:** bump @libp2p/interface-mocks from 8.0.5 to 9.1.1 ([libp2p#246](libp2p/js-libp2p-tcp#246)) ([6f01bd1](libp2p/js-libp2p-tcp@6f01bd1))
* **dev:** bump aegir from 37.12.1 to 38.1.0 ([libp2p#243](libp2p/js-libp2p-tcp#243)) ([e7eea28](libp2p/js-libp2p-tcp@e7eea28))
maschad added a commit to maschad/js-libp2p that referenced this issue Jun 21, 2023
@maschad
deps: add ws types to main dependencies (libp2p#246)
628e260 
While following the example of how to do a basic libp2p setup, if we only configure webSockets as our [transport](https://github.com/libp2p/js-libp2p/blob/master/doc/GETTING_STARTED.md#transports) the types will be undefined.

```bash
node_modules/@libp2p/websockets/dist/src/index.d.ts:5:36 - error TS7016: Could not find a declaration file for module 'ws'. '/Users/horizon/Desktop/test/node_modules/ws/index.js' implicitly has an 'any' type.
  Try `npm i --save-dev @types/ws` if it exists or add a new declaration (.d.ts) file containing `declare module 'ws';`

5 import type { ClientOptions } from 'ws';
                                     ~~~~
```

Perhaps these types should be exported in a better way? Especially since they are required in [it-ws](https://www.npmjs.com/package/it-ws) but if so then maybe the are not properly [exported there](https://github.com/alanshaw/it-ws/blob/master/package.json#L50)

```bash
node_modules/it-ws/dist/src/client.d.ts:1:36 - error TS7016: Could not find a declaration file for module 'ws'. '/Users/horizon/Desktop/test/node_modules/ws/index.js' implicitly has an 'any' type.
  Try `npm i --save-dev @types/ws` if it exists or add a new declaration (.d.ts) file containing `declare module 'ws';`

1 import type { ClientOptions } from 'ws';
                                     ~~~~

node_modules/it-ws/dist/src/duplex.d.ts:1:23 - error TS2688: Cannot find type definition file for 'ws'.

1 /// <reference types="ws" />
                        ~~

node_modules/it-ws/dist/src/sink.d.ts:1:32 - error TS7016: Could not find a declaration file for module 'ws'. '/Users/horizon/Desktop/test/node_modules/ws/index.js' implicitly has an 'any' type.
  Try `npm i --save-dev @types/ws` if it exists or add a new declaration (.d.ts) file containing `declare module 'ws';`

1 import type { WebSocket } from 'ws';
                                 ~~~~

node_modules/it-ws/dist/src/web-socket.d.ts:1:23 - error TS7016: Could not find a declaration file for module 'ws'. '/Users/horizon/Desktop/test/node_modules/ws/index.js' implicitly has an 'any' type.
  Try `npm i --save-dev @types/ws` if it exists or add a new declaration (.d.ts) file containing `declare module 'ws';`

1 import WebSocket from 'ws';
                        ~~~~
```
maschad pushed a commit to maschad/js-libp2p that referenced this issue Jun 21, 2023
@semantic-release-bot
chore(release): 6.0.2 [skip ci]
4549df7 
## [6.0.2](libp2p/js-libp2p-websockets@v6.0.1...v6.0.2) (2023-06-06)

### Dependencies

* add ws types to main dependencies ([libp2p#246](libp2p/js-libp2p-websockets#246)) ([628e260](libp2p/js-libp2p-websockets@628e260)), closes [/github.com/alanshaw/it-ws/blob/master/package.json#L50](https://github.com/libp2p//github.com/alanshaw/it-ws/blob/master/package.json/issues/L50)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
exp/novice Someone with a little familiarity can pick up good first issue Good issue for new contributors help wanted Seeking public contribution on this issue topic/docs Documentation
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

docs: add note about muxer ordering libp2p/js-libp2p
3 participants
@vmx @achingbrain @daviddias