chore: replace err-code with CodeError #82
Conversation
|
@wemeetagain |
|
@wemeetagain I think this solution is pretty good. There is a breaking change but might not be relevant. |
src/error.ts
Outdated
| export class OverStreamLimitError extends WebRTCTransportError { | ||
| constructor (msg: string) { | ||
| super(msg) | ||
| constructor (dir: Direction, proto: string) { | ||
| const code = dir === 'inbound' ? codes.ERR_TOO_MANY_INBOUND_PROTOCOL_STREAMS : codes.ERR_TOO_MANY_OUTBOUND_PROTOCOL_STREAMS | ||
| super(`${dir} stream limit reached for protocol - ${proto}`, code) | ||
| this.name = 'WebRTC/OverStreamLimitError' | ||
| } | ||
| } | ||
|
|
||
| export function overStreamLimit (dir: Direction, proto: string) { | ||
| const code = dir === 'inbound' ? codes.ERR_TOO_MANY_INBOUND_PROTOCOL_STREAMS : codes.ERR_TOO_MANY_OUTBOUND_PROTOCOL_STREAMS | ||
| return errCode(new OverStreamLimitError(`${dir} stream limit reached for protocol - ${proto}`), code) | ||
| return new OverStreamLimitError(dir, proto) | ||
| } |
There was a problem hiding this comment.
Breaking change has to do with exported OverStreamLimitError parameter change.
There was a problem hiding this comment.
Same comment here, we could have a default value for proto param in the OverStreamLimitError constructor (e.g. unknown). We could add TODOs and make the breaking changes with a planned major version increase.
There was a problem hiding this comment.
I can make proto in overStreamLimit optional; note this would be adding a change.
The parameter change I made has to do with the OverStreamLimitError which completely changes the parameters.
I'll undo the breaking parts and add a note.
|
Wondering if the Error returning Functions are necessary. They could just be removed as they don't do much now. |
src/error.ts
Outdated
| export class WebRTCTransportError extends Error { | ||
| constructor (msg: string) { | ||
| super(`WebRTC transport error: ${msg}`) | ||
| export class WebRTCTransportError extends CodeError { | ||
| constructor (msg: string, code: string) { | ||
| super(`WebRTC transport error: ${msg}`, code) | ||
| this.name = 'WebRTCTransportError' | ||
| } | ||
| } |
There was a problem hiding this comment.
Other breaking change
There was a problem hiding this comment.
Can we avoid the breaking change by making the parameter optional?
tabcat
force-pushed
the
chore/code-error
branch
from
6977f19 to
c340711
Compare
|
Lint is failing from a bug in esquery 1.4.1 estools/esquery#135 Do not rerun actions until fix is merged |
|
@tabcat I merged the aegir PR which I think has the fix for the esquery issue. |
|
@tabcat can you resolve the conflicts? I think Chinmay needed to update the return type related to the aegir update, so it's probably a comb on "yours" and "theirs". |
tabcat
force-pushed
the
chore/code-error
branch
2 times, most recently
from
e99350d to
5f62fa6
Compare
|
My mistake, I'll take care of this right now |
deps: remove err-code replace errcode with CodeError remove breaking changes fix linter errors
tabcat
force-pushed
the
chore/code-error
branch
from
5f62fa6 to
db28d7c
Compare
## [1.0.5](v1.0.4...v1.0.5) (2023-03-30) ### Bug Fixes * correction package.json exports types path ([#103](#103)) ([c78851f](c78851f)) ### Trivial Changes * replace err-code with CodeError ([#82](#82)) ([cfa6494](cfa6494)) * Update .github/workflows/semantic-pull-request.yml [skip ci] ([f0ae5e7](f0ae5e7)) * Update .github/workflows/semantic-pull-request.yml [skip ci] ([4c8806c](4c8806c))
|
🎉 This PR is included in version 1.0.5 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Related: libp2p/js-libp2p#1269