refactor!: move pnet into separate package #2165
Conversation
| psk: swarmKeyBuffer | ||
| })() | ||
| const protector: ConnectionProtector = { | ||
| async protect (connection) { |
There was a problem hiding this comment.
since this is just a fake, now, this is a more isolated test to connection-manager's use of protectors, which is good.
do we need to test that the arguments passed to connection protector are correct?
does the protector have tests to confirm it is doing the right thing since we're no longer confirming that as a side-effect of this test?
| @@ -3,3 +3,4 @@ export const INVALID_PSK = 'Your private shared key is invalid' | |||
| export const NO_LOCAL_ID = 'No local private key provided' | |||
| export const NO_HANDSHAKE_CONNECTION = 'No connection for the handshake provided' | |||
| export const STREAM_ENDED = 'Stream ended prematurely' | |||
| export const ERR_INVALID_PARAMETERS = 'ERR_INVALID_PARAMETERS' | |||
There was a problem hiding this comment.
should this and others be in @libp2p/interface ?
I know we had a discussion about this somewhere..
There was a problem hiding this comment.
Yes, some generic codes will reduce duplication but no need to block on this.
| @@ -81,7 +80,7 @@ class PreSharedKeyConnectionProtector implements ConnectionProtector { | |||
| } | |||
|
|
|||
| if (connection == null) { | |||
| throw new CodeError(Errors.NO_HANDSHAKE_CONNECTION, codes.ERR_INVALID_PARAMETERS) | |||
| throw new CodeError(Errors.NO_HANDSHAKE_CONNECTION, Errors.ERR_INVALID_PARAMETERS) | |||
There was a problem hiding this comment.
nit: we should normalize on errors|codes.ACTUAL_ERR and always import a singular object or always do ACTUAL_ERR
also(prob for outside this PR but..), where does CodeError come from, and can we enforce the error types there? if we did, we could use generics on modules and ensure each module has their possible error types enforced too (makes it easier for devs to know what errors can be thrown)
There was a problem hiding this comment.
where does CodeError come from
import { CodeError } from '@libp2p/interface/errors'
we could use generics
Maybe, I think this might get quite unwieldy. Feel free to open a PR to discuss?
| peerId: await createEd25519PeerId(), | ||
| peerRouting, | ||
| registrar, | ||
| addressManager, | ||
| connectionManager, | ||
| transportManager, |
There was a problem hiding this comment.
the preference for no trailing commas makes this diff so bad...
There was a problem hiding this comment.
cries in needless diff lines
cc @achingbrain
achingbrain
force-pushed
the
release-v1.0
branch
5 times, most recently
from
e831751
to
c347bf2
Compare
achingbrain
force-pushed
the
refactor/pnet
branch
2 times, most recently
from
ab4a4b8
to
4f4bc7a
Compare
achingbrain
force-pushed
the
release-v1.0
branch
from
c347bf2
to
c8b71d8
Compare
achingbrain
force-pushed
the
refactor/pnet
branch
3 times, most recently
from
0a74bf8
to
74d0522
Compare
achingbrain
force-pushed
the
refactor/pnet
branch
from
74d0522
to
07a0036
Compare
- Move pnet module to a separate package - Related to #1913
Description
Change checklist