Skip to content

Commit

Permalink
fix!: make PublishError.InsufficientPeers more self-explanatory (#487)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
achingbrain committed Feb 27, 2024
1 parent 42b5b92 commit 1958aab
Show file tree
Hide file tree
Showing 5 changed files with 25 additions and 10 deletions.
17 changes: 12 additions & 5 deletions src/index.ts
Expand Up @@ -105,8 +105,15 @@ export interface GossipsubOpts extends GossipsubOptsSpec, PubSubInit {
* reportMessageValidationResult() after the message is dropped from mcache won't forward the message.
*/
asyncValidation: boolean
/** Do not throw `InsufficientPeers` error if publishing to zero peers */
allowPublishToZeroPeers: boolean
/**
* Do not throw `PublishError.NoPeersSubscribedToTopic` error if there are no
* peers listening on the topic.
*
* N.B. if you sent this option to true, and you publish a message on a topic
* with no peers listening on that topic, no other network node will ever
* receive the message.
*/
allowPublishToZeroTopicPeers: boolean
/** Do not throw `PublishError.Duplicate` if publishing duplicate messages */
ignoreDuplicatePublishError: boolean
/** For a single stream, await processing each RPC before processing the next */
Expand Down Expand Up @@ -2137,10 +2144,10 @@ export class GossipSub extends TypedEventEmitter<GossipsubEvents> implements Pub
const willSendToSelf = this.opts.emitSelf && this.subscriptions.has(topic)

// Current publish opt takes precedence global opts, while preserving false value
const allowPublishToZeroPeers = opts?.allowPublishToZeroPeers ?? this.opts.allowPublishToZeroPeers
const allowPublishToZeroTopicPeers = opts?.allowPublishToZeroTopicPeers ?? this.opts.allowPublishToZeroTopicPeers

if (tosend.size === 0 && !allowPublishToZeroPeers && !willSendToSelf) {
throw Error('PublishError.InsufficientPeers')
if (tosend.size === 0 && !allowPublishToZeroTopicPeers && !willSendToSelf) {
throw Error('PublishError.NoPeersSubscribedToTopic')
}

// If the message isn't a duplicate and we have sent it to some peers add it to the
Expand Down
10 changes: 9 additions & 1 deletion src/types.ts
Expand Up @@ -71,7 +71,15 @@ export enum SignaturePolicy {
}

export interface PublishOpts {
allowPublishToZeroPeers?: boolean
/**
* Do not throw `PublishError.NoPeersSubscribedToTopic` error if there are no
* peers listening on the topic.
*
* N.B. if you sent this option to true, and you publish a message on a topic
* with no peers listening on that topic, no other network node will ever
* receive the message.
*/
allowPublishToZeroTopicPeers?: boolean
ignoreDuplicatePublishError?: boolean
/** serialize message once and send to all peers without control messages */
batchPublish?: boolean
Expand Down
2 changes: 1 addition & 1 deletion test/2-nodes.spec.ts
Expand Up @@ -243,7 +243,7 @@ describe('2 nodes', () => {
// Create pubsub nodes
beforeEach(async () => {
mockNetwork.reset()
nodes = await createComponentsArray({ number: 2, init: { allowPublishToZeroPeers: true } })
nodes = await createComponentsArray({ number: 2, init: { allowPublishToZeroTopicPeers: true } })
await connectAllPubSubNodes(nodes)

// Create subscriptions
Expand Down
2 changes: 1 addition & 1 deletion test/compliance.spec.ts
Expand Up @@ -29,7 +29,7 @@ describe.skip('interface compliance', function () {
...args.init,
// libp2p-interfaces-compliance-tests in test 'can subscribe and unsubscribe correctly' publishes to no peers
// Disable check to allow passing tests
allowPublishToZeroPeers: true
allowPublishToZeroTopicPeers: true
}
)

Expand Down
4 changes: 2 additions & 2 deletions test/gossip.spec.ts
Expand Up @@ -27,7 +27,7 @@ describe('gossip', () => {
IPColocationFactorThreshold: GossipsubDhi + 3
},
maxInboundDataLength: 4000000,
allowPublishToZeroPeers: false
allowPublishToZeroTopicPeers: false
}
})
})
Expand Down Expand Up @@ -89,7 +89,7 @@ describe('gossip', () => {
const topic = 'Z'

const publishResult = await nodeA.pubsub.publish(topic, uint8ArrayFromString('hey'), {
allowPublishToZeroPeers: true
allowPublishToZeroTopicPeers: true
})

// gossip happens during the heartbeat
Expand Down

0 comments on commit 1958aab

Please sign in to comment.