From 1958aabc1d0c77170a20d00c10fe416a8fed200f Mon Sep 17 00:00:00 2001
From: Alex Potsides <alex@achingbrain.net>
Date: Tue, 27 Feb 2024 16:43:13 +0000
Subject: [PATCH] fix!: make PublishError.InsufficientPeers more
 self-explanatory (#487)

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
---
 src/index.ts            | 17 ++++++++++++-----
 src/types.ts            | 10 +++++++++-
 test/2-nodes.spec.ts    |  2 +-
 test/compliance.spec.ts |  2 +-
 test/gossip.spec.ts     |  4 ++--
 5 files changed, 25 insertions(+), 10 deletions(-)

diff --git a/src/index.ts b/src/index.ts
index 3dd9d9ea..9bc059a6 100644
--- a/src/index.ts
+++ b/src/index.ts
@@ -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 */
@@ -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
diff --git a/src/types.ts b/src/types.ts
index d2b33b3b..8455eb4c 100644
--- a/src/types.ts
+++ b/src/types.ts
@@ -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
diff --git a/test/2-nodes.spec.ts b/test/2-nodes.spec.ts
index 97a710a9..82a3d9df 100644
--- a/test/2-nodes.spec.ts
+++ b/test/2-nodes.spec.ts
@@ -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
diff --git a/test/compliance.spec.ts b/test/compliance.spec.ts
index 5e3da4d4..d5e364ac 100644
--- a/test/compliance.spec.ts
+++ b/test/compliance.spec.ts
@@ -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
         }
       )
 
diff --git a/test/gossip.spec.ts b/test/gossip.spec.ts
index 11a09aa0..6c4cfeb7 100644
--- a/test/gossip.spec.ts
+++ b/test/gossip.spec.ts
@@ -27,7 +27,7 @@ describe('gossip', () => {
           IPColocationFactorThreshold: GossipsubDhi + 3
         },
         maxInboundDataLength: 4000000,
-        allowPublishToZeroPeers: false
+        allowPublishToZeroTopicPeers: false
       }
     })
   })
@@ -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