From b696a8957211bf20577f419a207a23ceca03d23f Mon Sep 17 00:00:00 2001 From: "fryorcraken.eth" Date: Wed, 20 Sep 2023 15:56:47 +1000 Subject: [PATCH 1/5] fix: catch stream creation promise rejection for `lightPush.send` --- packages/core/src/lib/light_push/index.ts | 22 +++++++++++++++++++--- packages/interfaces/src/protocols.ts | 3 ++- 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/packages/core/src/lib/light_push/index.ts b/packages/core/src/lib/light_push/index.ts index d9379c1690..4ecdea0be4 100644 --- a/packages/core/src/lib/light_push/index.ts +++ b/packages/core/src/lib/light_push/index.ts @@ -1,3 +1,4 @@ +import type { Stream } from "@libp2p/interface/connection"; import type { PeerId } from "@libp2p/interface/peer-id"; import { IEncoder, @@ -102,9 +103,24 @@ class LightPush extends BaseProtocol implements ILightPush { numPeers: this.NUM_PEERS_PROTOCOL }); + if (!peers.length) { + return { + recipients, + errors: [SendError.NO_PEER_AVAILABLE] + }; + } + const promises = peers.map(async (peer) => { let error: SendError | undefined; - const stream = await this.getStream(peer); + + let stream: Stream | undefined; + try { + stream = await this.getStream(peer); + } catch (err) { + log(`Failed to get a stream for remote peer${peer.id.toString()}`, err); + error = SendError.REMOTE_PEER_FAULT; + return { recipients, error }; + } try { const res = await pipe( @@ -126,8 +142,8 @@ class LightPush extends BaseProtocol implements ILightPush { recipients.some((recipient) => recipient.equals(peer.id)) || recipients.push(peer.id); } else { - log("No response in PushRPC"); - error = SendError.NO_RPC_RESPONSE; + log("Remote peer fault: No response in PushRPC"); + error = SendError.REMOTE_PEER_FAULT; } } catch (err) { log("Failed to decode push reply", err); diff --git a/packages/interfaces/src/protocols.ts b/packages/interfaces/src/protocols.ts index c354b1b740..aa98bef560 100644 --- a/packages/interfaces/src/protocols.ts +++ b/packages/interfaces/src/protocols.ts @@ -63,7 +63,8 @@ export enum SendError { ENCODE_FAILED = "Failed to encode", DECODE_FAILED = "Failed to decode", SIZE_TOO_BIG = "Size is too big", - NO_RPC_RESPONSE = "No RPC response" + NO_PEER_AVAILABLE = "No peer available", + REMOTE_PEER_FAULT = "Remote peer fault" } export interface SendResult { From a31b6e472e0087f017a938bb6858d53f2a37f1de Mon Sep 17 00:00:00 2001 From: "fryorcraken.eth" Date: Wed, 20 Sep 2023 16:02:52 +1000 Subject: [PATCH 2/5] refactor: Remove nest of try/catch in favor of sequential try/catch --- packages/core/src/lib/light_push/index.ts | 52 +++++++++++------------ 1 file changed, 26 insertions(+), 26 deletions(-) diff --git a/packages/core/src/lib/light_push/index.ts b/packages/core/src/lib/light_push/index.ts index 4ecdea0be4..de78c3b144 100644 --- a/packages/core/src/lib/light_push/index.ts +++ b/packages/core/src/lib/light_push/index.ts @@ -111,50 +111,50 @@ class LightPush extends BaseProtocol implements ILightPush { } const promises = peers.map(async (peer) => { - let error: SendError | undefined; - let stream: Stream | undefined; try { stream = await this.getStream(peer); } catch (err) { log(`Failed to get a stream for remote peer${peer.id.toString()}`, err); - error = SendError.REMOTE_PEER_FAULT; - return { recipients, error }; + return { recipients, error: SendError.REMOTE_PEER_FAULT }; } + let res: Uint8ArrayList[] | undefined; try { - const res = await pipe( + res = await pipe( [query.encode()], lp.encode, stream, lp.decode, async (source) => await all(source) ); - try { - const bytes = new Uint8ArrayList(); - res.forEach((chunk) => { - bytes.append(chunk); - }); - - const response = PushRpc.decode(bytes).response; - - if (response?.isSuccess) { - recipients.some((recipient) => recipient.equals(peer.id)) || - recipients.push(peer.id); - } else { - log("Remote peer fault: No response in PushRPC"); - error = SendError.REMOTE_PEER_FAULT; - } - } catch (err) { - log("Failed to decode push reply", err); - error = SendError.DECODE_FAILED; - } } catch (err) { log("Failed to send waku light push request", err); - error = SendError.GENERIC_FAIL; + return { recipients, error: SendError.GENERIC_FAIL }; } - return { recipients, error }; + const bytes = new Uint8ArrayList(); + res.forEach((chunk) => { + bytes.append(chunk); + }); + + let response: PushResponse | undefined; + try { + response = PushRpc.decode(bytes).response; + } catch (err) { + log("Failed to decode push reply", err); + return { recipients, error: SendError.DECODE_FAILED }; + } + + if (response?.isSuccess) { + recipients.some((recipient) => recipient.equals(peer.id)) || + recipients.push(peer.id); + } else { + log("Remote peer fault: No response in PushRPC"); + return { recipients, error: SendError.REMOTE_PEER_FAULT }; + } + + return { recipients }; }); const results = await Promise.allSettled(promises); From 6807185f3b8cdcad7b65cfda4482bb76dd5a4a08 Mon Sep 17 00:00:00 2001 From: "fryorcraken.eth" Date: Wed, 20 Sep 2023 16:18:22 +1000 Subject: [PATCH 3/5] doc: document potential errors --- packages/interfaces/src/protocols.ts | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/packages/interfaces/src/protocols.ts b/packages/interfaces/src/protocols.ts index aa98bef560..62daf66df2 100644 --- a/packages/interfaces/src/protocols.ts +++ b/packages/interfaces/src/protocols.ts @@ -59,11 +59,29 @@ export type Callback = ( ) => void | Promise; export enum SendError { + /** Could not determine the origin of the fault. Best to check connectivity and try again */ GENERIC_FAIL = "Generic error", + /** Failure to protobuf encode the message. This is not recoverable and needs + * further investigation. */ ENCODE_FAILED = "Failed to encode", + /** Failure to protobuf decode the message. May be due to a remote peer issue, + * ensuring that messages are sent via several peer enable mitigation of this error.. */ DECODE_FAILED = "Failed to decode", + /** The message size is above the maximum message size allowed on the Waku Network. + * Compressing the message or using an alternative strategy for large messages is recommended. + */ SIZE_TOO_BIG = "Size is too big", + /** + * Failure to find a peer with suitable protocols. This may due to a connection issue. + * Mitigation can be: retrying after a given time period, display connectivity issue + * to user or listening for `peer:connected:bootstrap` or `peer:connected:peer-exchange` + * on the connection manager before retrying. + */ NO_PEER_AVAILABLE = "No peer available", + /** + * The remote peer did not behave as expected. Mitigation from `NO_PEER_AVAILABLE` + * or `DECODE_FAILED` can be used. + */ REMOTE_PEER_FAULT = "Remote peer fault" } From 053b6545ad0c2450af5687495eb7b6049c0f21ad Mon Sep 17 00:00:00 2001 From: "fryorcraken.eth" Date: Thu, 21 Sep 2023 11:32:34 +1000 Subject: [PATCH 4/5] feat!: return `REMOTE_PEER_REJECTED` if remote peer rejected the message --- packages/core/src/lib/light_push/index.ts | 13 +++++++++---- packages/interfaces/src/protocols.ts | 10 ++++++++-- packages/tests/tests/light-push/index.spec.ts | 4 ++-- 3 files changed, 19 insertions(+), 8 deletions(-) diff --git a/packages/core/src/lib/light_push/index.ts b/packages/core/src/lib/light_push/index.ts index de78c3b144..f4ffcb7fc2 100644 --- a/packages/core/src/lib/light_push/index.ts +++ b/packages/core/src/lib/light_push/index.ts @@ -146,14 +146,19 @@ class LightPush extends BaseProtocol implements ILightPush { return { recipients, error: SendError.DECODE_FAILED }; } - if (response?.isSuccess) { - recipients.some((recipient) => recipient.equals(peer.id)) || - recipients.push(peer.id); - } else { + if (!response) { log("Remote peer fault: No response in PushRPC"); return { recipients, error: SendError.REMOTE_PEER_FAULT }; } + if (!response.isSuccess) { + log("Remote peer rejected the message: ", response.info); + return { recipients, error: SendError.REMOTE_PEER_REJECTED }; + } + + recipients.some((recipient) => recipient.equals(peer.id)) || + recipients.push(peer.id); + return { recipients }; }); diff --git a/packages/interfaces/src/protocols.ts b/packages/interfaces/src/protocols.ts index 62daf66df2..d84172f6e7 100644 --- a/packages/interfaces/src/protocols.ts +++ b/packages/interfaces/src/protocols.ts @@ -79,10 +79,16 @@ export enum SendError { */ NO_PEER_AVAILABLE = "No peer available", /** - * The remote peer did not behave as expected. Mitigation from `NO_PEER_AVAILABLE` + * The remote peer did not behave as expected. Mitigation for `NO_PEER_AVAILABLE` * or `DECODE_FAILED` can be used. */ - REMOTE_PEER_FAULT = "Remote peer fault" + REMOTE_PEER_FAULT = "Remote peer fault", + /** + * The remote peer rejected the message. Information provided by the remote peer + * is logged. Review message validity, or mitigation for `NO_PEER_AVAILABLE` + * or `DECODE_FAILED` can be used. + */ + REMOTE_PEER_REJECTED = "Remote peer rejected" } export interface SendResult { diff --git a/packages/tests/tests/light-push/index.spec.ts b/packages/tests/tests/light-push/index.spec.ts index 3334cd76fb..5281cb1222 100644 --- a/packages/tests/tests/light-push/index.spec.ts +++ b/packages/tests/tests/light-push/index.spec.ts @@ -86,7 +86,7 @@ describe("Waku Light Push [node only]", function () { }); } else { expect(pushResponse.recipients.length).to.eq(0); - expect(pushResponse.errors).to.include(SendError.NO_RPC_RESPONSE); + expect(pushResponse.errors).to.include(SendError.REMOTE_PEER_REJECTED); expect(await messageCollector.waitForMessages(1)).to.eq(false); } }); @@ -158,7 +158,7 @@ describe("Waku Light Push [node only]", function () { }); } else { expect(pushResponse.recipients.length).to.eq(0); - expect(pushResponse.errors).to.include(SendError.NO_RPC_RESPONSE); + expect(pushResponse.errors).to.include(SendError.REMOTE_PEER_REJECTED); expect(await messageCollector.waitForMessages(1)).to.eq(false); } }); From feb6e1b92cb9210da944cc459098a012f39dae54 Mon Sep 17 00:00:00 2001 From: "fryorcraken.eth" Date: Thu, 21 Sep 2023 12:25:38 +1000 Subject: [PATCH 5/5] test: nwaku does not behave as expected --- packages/tests/tests/light-push/index.spec.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/tests/tests/light-push/index.spec.ts b/packages/tests/tests/light-push/index.spec.ts index 5281cb1222..e43207bd9f 100644 --- a/packages/tests/tests/light-push/index.spec.ts +++ b/packages/tests/tests/light-push/index.spec.ts @@ -86,7 +86,8 @@ describe("Waku Light Push [node only]", function () { }); } else { expect(pushResponse.recipients.length).to.eq(0); - expect(pushResponse.errors).to.include(SendError.REMOTE_PEER_REJECTED); + // This should be `REMOTE_PEER_REJECTED`, tracked with https://github.com/waku-org/nwaku/issues/1641 + expect(pushResponse.errors).to.include(SendError.REMOTE_PEER_FAULT); expect(await messageCollector.waitForMessages(1)).to.eq(false); } }); @@ -158,7 +159,8 @@ describe("Waku Light Push [node only]", function () { }); } else { expect(pushResponse.recipients.length).to.eq(0); - expect(pushResponse.errors).to.include(SendError.REMOTE_PEER_REJECTED); + // Should be `REMOTE_PEER_REJECTED`, tracked with https://github.com/waku-org/nwaku/issues/2059 + expect(pushResponse.errors).to.include(SendError.REMOTE_PEER_FAULT); expect(await messageCollector.waitForMessages(1)).to.eq(false); } });