ChainSafe / js-libp2p-gossipsub

TypeScript implementation of Gossipsub
Apache License 2.0
145 stars 43 forks source link

fix!: make PublishError.InsufficientPeers more self-explanatory #487

Closed achingbrain closed 6 months ago

achingbrain commented 7 months ago

People continually mistake the InsufficientPeers error for something that can be "worked around" by setting allowPublishToZeroPeers to true, and they then expect other network nodes to still recieve their messages.

This PR renames the option to allowPublishToZeroTopicPeers - this makes it clear(er) that it's not that you don't have any peers, it's that no peers you have are listening on the topic.

It also improves the JSDoc comment on the option and changes the Error message from PublishError.InsufficientPeers to PublishError.NoPeersSubscribedToTopic which (I hope) more accurately describes what's wrong.

BREAKING CHANGE: The allowPublishToZeroPeers option has been renamed to allowPublishToZeroTopicPeers

Fixes #472

achingbrain commented 7 months ago

I'm not sure this is worth a major bump on it's own, maybe it should be batched up with #468?

codecov-commenter commented 7 months ago

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (b4e6a8d) 81.40% compared to head (435e8dd) 81.42%.

Files Patch % Lines
src/index.ts 91.66% 1 Missing :warning:
test/compliance.spec.ts 0.00% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #487 +/- ## ========================================== + Coverage 81.40% 81.42% +0.02% ========================================== Files 48 48 Lines 12325 12340 +15 Branches 1301 1301 ========================================== + Hits 10033 10048 +15 Misses 2292 2292 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.