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

abstract-stream: should support undefined logger #2275

Closed
paralin opened this issue Dec 1, 2023 · 3 comments
Closed

abstract-stream: should support undefined logger #2275

paralin opened this issue Dec 1, 2023 · 3 comments
Labels
need/author-input Needs input from the original author need/triage Needs initial labeling and prioritization

Comments

@paralin
Copy link
Contributor

paralin commented Dec 1, 2023

Abstract stream currently requires a non-undefined logger:

In previous releases of js-libp2p-yamux we could pass an empty components object without a logger, but this is not possible anymore as Log is required for AbstractStream: ChainSafe/js-libp2p-yamux#69

Please make log an optional parameter for AbstractStreamInit, maybe using a no-op shim internally if necessary (when it's undefined), so that the construct for yamux()() is backwards-compatible and doesn't need to have a Logger passed every time.

The same is true for MuxerInit: it appears to now require a not-null Logger at construct time.

@paralin paralin added the need/triage Needs initial labeling and prioritization label Dec 1, 2023
paralin added a commit to aperturerobotics/starpc that referenced this issue Dec 1, 2023
Update dependencies and fix various breaking changes introduced by those
dependency updates:

 - Change type of Conn to Duplex<Uint8Array | Uint8ArrayList>
 - Pass a logger stub to yamux as it's now required.

The logger stub should not be necessary once these are fixed:

- ChainSafe/js-libp2p-yamux#69
- libp2p/js-libp2p#2275
- ChainSafe/js-libp2p-yamux#70

Signed-off-by: Christian Stewart <christian@aperture.us>
paralin added a commit to aperturerobotics/starpc that referenced this issue Dec 1, 2023
NOTE: The following issues must be addressed before this is merged:

libp2p/interface is currently held at version 0.1.2:

 - Passing a logger to MuxerInit is now required (should be optional?): libp2p/js-libp2p#2275

@chainsafe/libp2p-yamux is currently held at version 5.0.4:

 - Passing a logger to yamux()({logger:...}) is now required (should be optional?): ChainSafe/js-libp2p-yamux#69
 - The integration test for rpcstream fails with 6.x vs. 5.x: ChainSafe/js-libp2p-yamux#71 and https://github.com/aperturerobotics/starpc/actions/runs/7055030516/job/19204891175

See: #120

---

Update dependencies and fix various breaking changes introduced by those
dependency updates:

- Change type of Conn to Duplex<Uint8Array | Uint8ArrayList>
- Pass a logger stub to yamux as it's now required.

The logger stub should not be necessary once these are fixed:

- ChainSafe/js-libp2p-yamux#69
- libp2p/js-libp2p#2275
- ChainSafe/js-libp2p-yamux#70

Signed-off-by: Christian Stewart <christian@aperture.us>
@achingbrain
Copy link
Member

When libp2p configures these components it passes a logging implementation in which is why it's not optional anywhere.

Could you speak a bit more to your use case? Are you doing something different here?

@achingbrain achingbrain added need/author-input Needs input from the original author need/triage Needs initial labeling and prioritization and removed need/triage Needs initial labeling and prioritization labels Dec 1, 2023
@paralin
Copy link
Contributor Author

paralin commented Dec 1, 2023

I'm using js-libp2p-yamux externally from libp2p and don't want to pass a logger.

paralin added a commit to aperturerobotics/starpc that referenced this issue Dec 11, 2023
NOTE: The following issues must be addressed before this is merged:

libp2p/interface is currently held at version 0.1.2:

 - Passing a logger to MuxerInit is now required (should be optional?): libp2p/js-libp2p#2275

@chainsafe/libp2p-yamux is currently held at version 5.0.4:

 - Passing a logger to yamux()({logger:...}) is now required (should be optional?): ChainSafe/js-libp2p-yamux#69
 - The integration test for rpcstream fails with 6.x vs. 5.x: ChainSafe/js-libp2p-yamux#71 and https://github.com/aperturerobotics/starpc/actions/runs/7055030516/job/19204891175

See: #120

---

Update dependencies and fix various breaking changes introduced by those
dependency updates:

- Change type of Conn to Duplex<Uint8Array | Uint8ArrayList>
- Pass a logger stub to yamux as it's now required.

The logger stub should not be necessary once these are fixed:

- ChainSafe/js-libp2p-yamux#69
- libp2p/js-libp2p#2275
- ChainSafe/js-libp2p-yamux#70

Signed-off-by: Christian Stewart <christian@aperture.us>
paralin added a commit to aperturerobotics/starpc that referenced this issue Dec 19, 2023
paralin added a commit to aperturerobotics/starpc that referenced this issue Jan 10, 2024
NOTE: The following issues must be addressed before this is merged:

libp2p/interface is currently held at version 0.1.2:

 - Passing a logger to MuxerInit is now required (should be optional?): libp2p/js-libp2p#2275

@chainsafe/libp2p-yamux is currently held at version 5.0.4:

 - Passing a logger to yamux()({logger:...}) is now required (should be optional?): ChainSafe/js-libp2p-yamux#69
 - The integration test for rpcstream fails with 6.x vs. 5.x: ChainSafe/js-libp2p-yamux#71 and https://github.com/aperturerobotics/starpc/actions/runs/7055030516/job/19204891175

See: #120

---

Update dependencies and fix various breaking changes introduced by those
dependency updates:

- Change type of Conn to Duplex<Uint8Array | Uint8ArrayList>
- Pass a logger stub to yamux as it's now required.

The logger stub should not be necessary once these are fixed:

- ChainSafe/js-libp2p-yamux#69
- libp2p/js-libp2p#2275
- ChainSafe/js-libp2p-yamux#70

Signed-off-by: Christian Stewart <christian@aperture.us>
@paralin
Copy link
Contributor Author

paralin commented Jan 10, 2024

I give up on this and will just pass a stub Log from now on.

@paralin paralin closed this as completed Jan 10, 2024
paralin added a commit to aperturerobotics/starpc that referenced this issue Jan 10, 2024
 - Passing a logger to MuxerInit is now required
 - Passing a logger to yamux()({logger:...}) is now required

See: #120

Update dependencies and fix various breaking changes introduced by those
dependency updates:

- Change type of Conn to Duplex<Uint8Array | Uint8ArrayList>
- Pass a logger stub to yamux as it's now required.

Related:

- ChainSafe/js-libp2p-yamux#69
- libp2p/js-libp2p#2275
- ChainSafe/js-libp2p-yamux#70

Signed-off-by: Christian Stewart <christian@aperture.us>
paralin added a commit to aperturerobotics/starpc that referenced this issue Jan 17, 2024
 - Passing a logger to MuxerInit is now required
 - Passing a logger to yamux()({logger:...}) is now required

See: #120

Update dependencies and fix various breaking changes introduced by those
dependency updates:

- Change type of Conn to Duplex<Uint8Array | Uint8ArrayList>
- Pass a logger stub to yamux as it's now required.

Related:

- ChainSafe/js-libp2p-yamux#69
- libp2p/js-libp2p#2275
- ChainSafe/js-libp2p-yamux#70

Signed-off-by: Christian Stewart <christian@aperture.us>
paralin added a commit to aperturerobotics/starpc that referenced this issue Jan 17, 2024
 - Passing a logger to MuxerInit is now required
 - Passing a logger to yamux()({logger:...}) is now required

See: #120

Update dependencies and fix various breaking changes introduced by those
dependency updates:

- Change type of Conn to Duplex<Uint8Array | Uint8ArrayList>
- Pass a logger stub to yamux as it's now required.

Related:

- ChainSafe/js-libp2p-yamux#69
- libp2p/js-libp2p#2275
- ChainSafe/js-libp2p-yamux#70

Signed-off-by: Christian Stewart <christian@aperture.us>
paralin added a commit to aperturerobotics/starpc that referenced this issue Feb 7, 2024
 - Passing a logger to MuxerInit is now required
 - Passing a logger to yamux()({logger:...}) is now required

See: #120

Update dependencies and fix various breaking changes introduced by those
dependency updates:

- Change type of Conn to Duplex<Uint8Array | Uint8ArrayList>
- Pass a logger stub to yamux as it's now required.

Related:

- ChainSafe/js-libp2p-yamux#69
- libp2p/js-libp2p#2275
- ChainSafe/js-libp2p-yamux#70

Signed-off-by: Christian Stewart <christian@aperture.us>
paralin added a commit to aperturerobotics/starpc that referenced this issue Feb 7, 2024
 - Passing a logger to MuxerInit is now required
 - Passing a logger to yamux()({logger:...}) is now required

See: #120

Update dependencies and fix various breaking changes introduced by those
dependency updates:

- Change type of Conn to Duplex<Uint8Array | Uint8ArrayList>
- Pass default logger to yamux as it's now required.

Related:

- ChainSafe/js-libp2p-yamux#69
- libp2p/js-libp2p#2275
- ChainSafe/js-libp2p-yamux#70

Signed-off-by: Christian Stewart <christian@aperture.us>
paralin added a commit to aperturerobotics/starpc that referenced this issue Feb 9, 2024
 - Passing a logger to MuxerInit is now required
 - Passing a logger to yamux()({logger:...}) is now required

See: #120

Update dependencies and fix various breaking changes introduced by those
dependency updates:

- Change type of Conn to Duplex<Uint8Array | Uint8ArrayList>
- Pass default logger to yamux as it's now required.

Related:

- ChainSafe/js-libp2p-yamux#69
- libp2p/js-libp2p#2275
- ChainSafe/js-libp2p-yamux#70

Signed-off-by: Christian Stewart <christian@aperture.us>
paralin added a commit to aperturerobotics/starpc that referenced this issue Feb 10, 2024
 - Passing a logger to MuxerInit is now required
 - Passing a logger to yamux()({logger:...}) is now required

See: #120

Update dependencies and fix various breaking changes introduced by those
dependency updates:

- Change type of Conn to Duplex<Uint8Array | Uint8ArrayList>
- Pass default logger to yamux as it's now required.
 - We need to pass a Uint8ArrayList to the WebSocket it-ws.

Related:

- ChainSafe/js-libp2p-yamux#69
- libp2p/js-libp2p#2275
- ChainSafe/js-libp2p-yamux#70

Signed-off-by: Christian Stewart <christian@aperture.us>
paralin added a commit to aperturerobotics/starpc that referenced this issue Feb 10, 2024
 - Passing a logger to MuxerInit is now required
 - Passing a logger to yamux()({logger:...}) is now required

See: #120

Update dependencies and fix various breaking changes introduced by those
dependency updates:

- Change type of Conn to Duplex<Uint8Array | Uint8ArrayList>
- Pass default logger to yamux as it's now required.
 - We need to pass a Uint8ArrayList to the WebSocket it-ws.

Related:

- ChainSafe/js-libp2p-yamux#69
- libp2p/js-libp2p#2275
- ChainSafe/js-libp2p-yamux#70

Signed-off-by: Christian Stewart <christian@aperture.us>
paralin added a commit to aperturerobotics/starpc that referenced this issue Feb 10, 2024
 - Passing a logger to MuxerInit is now required
 - Passing a logger to yamux()({logger:...}) is now required

See: #120

Update dependencies and fix various breaking changes introduced by those
dependency updates:

- Change type of Conn to Duplex<Uint8Array | Uint8ArrayList>
- Pass default logger to yamux as it's now required.
 - We need to pass a Uint8ArrayList to the WebSocket it-ws.

Related:

- ChainSafe/js-libp2p-yamux#69
- libp2p/js-libp2p#2275
- ChainSafe/js-libp2p-yamux#70

Signed-off-by: Christian Stewart <christian@aperture.us>
paralin added a commit to aperturerobotics/starpc that referenced this issue Feb 10, 2024
 - Passing a logger to MuxerInit is now required
 - Passing a logger to yamux()({logger:...}) is now required

See: #120

Update dependencies and fix various breaking changes introduced by those
dependency updates:

- Change type of Conn to Duplex<Uint8Array | Uint8ArrayList>
- Pass default logger to yamux as it's now required.
 - We need to pass a Uint8ArrayList to the WebSocket it-ws.

Related:

- ChainSafe/js-libp2p-yamux#69
- libp2p/js-libp2p#2275
- ChainSafe/js-libp2p-yamux#70

Signed-off-by: Christian Stewart <christian@aperture.us>
paralin added a commit to aperturerobotics/starpc that referenced this issue Feb 10, 2024
 - Passing a logger to MuxerInit is now required
 - Passing a logger to yamux()({logger:...}) is now required

See: #120

Update dependencies and fix various breaking changes introduced by those
dependency updates:

- Change type of Conn to Duplex<Uint8Array | Uint8ArrayList>
- Pass default logger to yamux as it's now required.
 - We need to pass a Uint8ArrayList to the WebSocket it-ws.

Related:

- ChainSafe/js-libp2p-yamux#69
- libp2p/js-libp2p#2275
- ChainSafe/js-libp2p-yamux#70

Signed-off-by: Christian Stewart <christian@aperture.us>
paralin added a commit to aperturerobotics/starpc that referenced this issue Feb 10, 2024
 - Passing a logger to MuxerInit is now required
 - Passing a logger to yamux()({logger:...}) is now required

See: #120

Update dependencies and fix various breaking changes introduced by those
dependency updates:

- Change type of Conn to Duplex<Uint8Array | Uint8ArrayList>
- Pass default logger to yamux as it's now required.
 - We need to pass a Uint8ArrayList to the WebSocket it-ws.

Related:

- ChainSafe/js-libp2p-yamux#69
- libp2p/js-libp2p#2275
- ChainSafe/js-libp2p-yamux#70

Signed-off-by: Christian Stewart <christian@aperture.us>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
need/author-input Needs input from the original author need/triage Needs initial labeling and prioritization
Projects
None yet
Development

No branches or pull requests

2 participants