waku-org / js-waku

JavaScript implementation of Waku v2
https://js.waku.org
Apache License 2.0
162 stars 41 forks source link

feat: mapping `statusCode` to tangible errors #1999

Open danisharora099 opened 2 months ago

danisharora099 commented 2 months ago

This is a feature request

Problem

For protocol implementations (eg Filter), we have access to certain statusCode that the remote peer might respond with. Based on specific statusCode, returning and handling errors appropriately would be great devex.

Proposed Solutions

Map error codes received from remote peers into tangible errors

Notes

We currently do follow generic error handling based on statusCode but the aim is to do it for more precise error codes:

 const { statusCode, requestId, statusDesc } =
      FilterSubscribeResponse.decode(res[0].slice());

    if (statusCode < 200 || statusCode >= 300) {
      log.error(
        `Filter unsubscribe all request ${requestId} failed with status code ${statusCode}: ${statusDesc}`
      );
      return {
        failure: {
          error: ProtocolError.REMOTE_PEER_REJECTED,
          peerId: peer.id
        },
        success: null
      };
    }
weboko commented 2 months ago

I believe we need input from nwaku what statusCode's are possible and what they mean

@waku-org/nwaku-developers

jm-clius commented 2 months ago

For Filter, the following enum is a good starting point: https://github.com/waku-org/nwaku/blob/91c85738a0e45f9bf88d11bc1b36308755a5b5d0/waku/waku_filter_v2/common.nim#L13-L19

(The idea is that when the protocol moves to stable, the error codes will be specified. For now the list is quite dynamic as more and more error cases are added.)

danisharora099 commented 2 months ago

For Filter, the following enum is a good starting point: https://github.com/waku-org/nwaku/blob/91c85738a0e45f9bf88d11bc1b36308755a5b5d0/waku/waku_filter_v2/common.nim#L13-L19

(The idea is that when the protocol moves to stable, the error codes will be specified. For now the list is quite dynamic as more and more error cases are added.)

Thanks @jm-clius! Do we also plan to introduce such error codes for other protocols?

jm-clius commented 2 months ago

Indeed. For each of the "new" req-resp protocol the pattern should be the same - an error enum in a common.nim. This has been implemented for store v3: https://github.com/waku-org/nwaku/blob/22f64bbd447985e375b9cb7e0302448fcd7f6636/waku/waku_store/common.nim#L61-L67

Lightpush and Peer-Exchange are still on the old versions of the protocol (i.e. they don't have status codes). If we determine a more compelling need for upgrading these protocols (as we did for Filter and Store) they will follow the same pattern.