js-waku icon indicating copy to clipboard operation
js-waku copied to clipboard

feat: mapping `statusCode` to tangible errors

Open danisharora099 opened this issue 9 months ago • 4 comments

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
      };
    }

danisharora099 avatar May 07 '24 07:05 danisharora099

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

@waku-org/nwaku-developers

weboko avatar May 07 '24 09:05 weboko

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.)

jm-clius avatar May 07 '24 09:05 jm-clius

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?

danisharora099 avatar May 07 '24 12:05 danisharora099

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.

jm-clius avatar May 07 '24 13:05 jm-clius