From 3133aaaf71a5feb0c30bcdd4638f320514b239e6 Mon Sep 17 00:00:00 2001 From: fryorcraken <110212804+fryorcraken@users.noreply.github.com> Date: Thu, 10 Jul 2025 10:56:02 +1000 Subject: [PATCH] chore: use distinct type for Light push status codes (#3488) * chore: use distinct type for Light push status codes * Make naming more explicit * test: use new light push error code in tests * fix missed line * fix thing --- tests/node/test_wakunode_lightpush.nim | 6 +-- tests/waku_lightpush/test_client.nim | 15 +++---- tests/waku_lightpush/test_ratelimit.nim | 2 +- waku/node/waku_node.nim | 16 +++++--- waku/waku_api/rest/lightpush/handlers.nim | 2 +- waku/waku_lightpush/callbacks.nim | 4 +- waku/waku_lightpush/client.nim | 11 +++-- waku/waku_lightpush/common.nim | 49 ++++++++++++----------- waku/waku_lightpush/protocol.nim | 20 +++++---- waku/waku_lightpush/rpc.nim | 6 ++- waku/waku_lightpush/rpc_codec.nim | 4 +- 11 files changed, 73 insertions(+), 62 deletions(-) diff --git a/tests/node/test_wakunode_lightpush.nim b/tests/node/test_wakunode_lightpush.nim index 6a42c899b..ee68710d1 100644 --- a/tests/node/test_wakunode_lightpush.nim +++ b/tests/node/test_wakunode_lightpush.nim @@ -76,7 +76,7 @@ suite "Waku Lightpush - End To End": # Then the message is not relayed but not due to RLN assert publishResponse.isErr(), "We expect an error response" - assert (publishResponse.error.code == NO_PEERS_TO_RELAY), + assert (publishResponse.error.code == LightPushErrorCode.NO_PEERS_TO_RELAY), "incorrect error response" suite "Waku LightPush Validation Tests": @@ -93,7 +93,7 @@ suite "Waku Lightpush - End To End": check: publishResponse.isErr() - publishResponse.error.code == INVALID_MESSAGE_ERROR + publishResponse.error.code == LightPushErrorCode.INVALID_MESSAGE publishResponse.error.desc == some(fmt"Message size exceeded maximum of {DefaultMaxWakuMessageSize} bytes") @@ -168,7 +168,7 @@ suite "RLN Proofs as a Lightpush Service": # Then the message is not relayed but not due to RLN assert publishResponse.isErr(), "We expect an error response" - check publishResponse.error.code == NO_PEERS_TO_RELAY + check publishResponse.error.code == LightPushErrorCode.NO_PEERS_TO_RELAY suite "Waku Lightpush message delivery": asyncTest "lightpush message flow succeed": diff --git a/tests/waku_lightpush/test_client.nim b/tests/waku_lightpush/test_client.nim index d7a1b6b24..af22ffa5d 100644 --- a/tests/waku_lightpush/test_client.nim +++ b/tests/waku_lightpush/test_client.nim @@ -42,8 +42,9 @@ suite "Waku Lightpush Client": ): Future[WakuLightPushResult] {.async.} = let msgLen = message.encode().buffer.len if msgLen > int(DefaultMaxWakuMessageSize) + 64 * 1024: - return - lighpushErrorResult(PAYLOAD_TOO_LARGE, "length greater than maxMessageSize") + return lighpushErrorResult( + LightPushErrorCode.PAYLOAD_TOO_LARGE, "length greater than maxMessageSize" + ) handlerFuture.complete((pubsubTopic, message)) # return that we published the message to 1 peer. return ok(1) @@ -263,7 +264,7 @@ suite "Waku Lightpush Client": # Then the message is not received by the server check: publishResponse5.isErr() - publishResponse5.error.code == PAYLOAD_TOO_LARGE + publishResponse5.error.code == LightPushErrorCode.PAYLOAD_TOO_LARGE (await handlerFuture.waitForResult()).isErr() asyncTest "Invalid Encoding Payload": @@ -276,7 +277,7 @@ suite "Waku Lightpush Client": # And the error is returned check: publishResponse.requestId == "N/A" - publishResponse.statusCode == LightpushStatusCode.BAD_REQUEST.uint32 + publishResponse.statusCode == LightPushErrorCode.BAD_REQUEST publishResponse.statusDesc.isSome() scanf(publishResponse.statusDesc.get(), decodeRpcFailure) @@ -289,7 +290,7 @@ suite "Waku Lightpush Client": peer: PeerId, pubsubTopic: PubsubTopic, message: WakuMessage ): Future[WakuLightPushResult] {.async.} = handlerFuture2.complete() - return lighpushErrorResult(PAYLOAD_TOO_LARGE, handlerError) + return lighpushErrorResult(LightPushErrorCode.PAYLOAD_TOO_LARGE, handlerError) let serverSwitch2 = newTestSwitch() @@ -305,7 +306,7 @@ suite "Waku Lightpush Client": # Then the response is negative check: - publishResponse.error.code == PAYLOAD_TOO_LARGE + publishResponse.error.code == LightPushErrorCode.PAYLOAD_TOO_LARGE publishResponse.error.desc == some(handlerError) (await handlerFuture2.waitForResult()).isOk() @@ -369,4 +370,4 @@ suite "Waku Lightpush Client": # Then the response is negative check not publishResponse.isOk() - check publishResponse.error.code == LightpushStatusCode.NO_PEERS_TO_RELAY + check publishResponse.error.code == LightPushErrorCode.NO_PEERS_TO_RELAY diff --git a/tests/waku_lightpush/test_ratelimit.nim b/tests/waku_lightpush/test_ratelimit.nim index 0dd7913d1..b2dcdc7b5 100644 --- a/tests/waku_lightpush/test_ratelimit.nim +++ b/tests/waku_lightpush/test_ratelimit.nim @@ -119,7 +119,7 @@ suite "Rate limited push service": check: requestRes.isErr() - requestRes.error.code == TOO_MANY_REQUESTS + requestRes.error.code == LightPushErrorCode.TOO_MANY_REQUESTS requestRes.error.desc == some(TooManyRequestsMessage) for testCnt in 0 .. 2: diff --git a/waku/node/waku_node.nim b/waku/node/waku_node.nim index b507b385e..db689e8a0 100644 --- a/waku/node/waku_node.nim +++ b/waku/node/waku_node.nim @@ -1191,7 +1191,9 @@ proc lightpushPublish*( ): Future[lightpush_protocol.WakuLightPushResult] {.async.} = if node.wakuLightpushClient.isNil() and node.wakuLightPush.isNil(): error "failed to publish message as lightpush not available" - return lighpushErrorResult(SERVICE_NOT_AVAILABLE, "Waku lightpush not available") + return lighpushErrorResult( + LightPushErrorCode.SERVICE_NOT_AVAILABLE, "Waku lightpush not available" + ) let toPeer: RemotePeerInfo = peerOpt.valueOr: if not node.wakuLightPush.isNil(): @@ -1200,25 +1202,27 @@ proc lightpushPublish*( node.peerManager.selectPeer(WakuLightPushCodec).valueOr: let msg = "no suitable remote peers" error "failed to publish message", msg = msg - return lighpushErrorResult(NO_PEERS_TO_RELAY, msg) + return lighpushErrorResult(LightPushErrorCode.NO_PEERS_TO_RELAY, msg) else: - return lighpushErrorResult(NO_PEERS_TO_RELAY, "no suitable remote peers") + return lighpushErrorResult( + LightPushErrorCode.NO_PEERS_TO_RELAY, "no suitable remote peers" + ) let pubsubForPublish = pubSubTopic.valueOr: if node.wakuAutoSharding.isNone(): let msg = "Pubsub topic must be specified when static sharding is enabled" error "lightpush publish error", error = msg - return lighpushErrorResult(INVALID_MESSAGE_ERROR, msg) + return lighpushErrorResult(LightPushErrorCode.INVALID_MESSAGE, msg) let parsedTopic = NsContentTopic.parse(message.contentTopic).valueOr: let msg = "Invalid content-topic:" & $error error "lightpush request handling error", error = msg - return lighpushErrorResult(INVALID_MESSAGE_ERROR, msg) + return lighpushErrorResult(LightPushErrorCode.INVALID_MESSAGE, msg) node.wakuAutoSharding.get().getShard(parsedTopic).valueOr: let msg = "Autosharding error: " & error error "lightpush publish error", error = msg - return lighpushErrorResult(INTERNAL_SERVER_ERROR, msg) + return lighpushErrorResult(LightPushErrorCode.INTERNAL_SERVER_ERROR, msg) return await lightpushPublishHandler(node, pubsubForPublish, message, toPeer) diff --git a/waku/waku_api/rest/lightpush/handlers.nim b/waku/waku_api/rest/lightpush/handlers.nim index cafcd89d2..a724aa1c9 100644 --- a/waku/waku_api/rest/lightpush/handlers.nim +++ b/waku/waku_api/rest/lightpush/handlers.nim @@ -32,7 +32,7 @@ const NoPeerNoneFoundError = "No suitable service peer & none discovered" proc useSelfHostedLightPush(node: WakuNode): bool = return node.wakuLightPush != nil and node.wakuLightPushClient == nil -proc convertErrorKindToHttpStatus(statusCode: LightpushStatusCode): HttpCode = +proc convertErrorKindToHttpStatus(statusCode: LightPushStatusCode): HttpCode = ## Lightpush status codes are matching HTTP status codes by design return toHttpCode(statusCode.int).get(Http500) diff --git a/waku/waku_lightpush/callbacks.nim b/waku/waku_lightpush/callbacks.nim index 3cfc3fe90..4b362e6bb 100644 --- a/waku/waku_lightpush/callbacks.nim +++ b/waku/waku_lightpush/callbacks.nim @@ -44,10 +44,10 @@ proc getRelayPushHandler*( ): Future[WakuLightPushResult] {.async.} = # append RLN proof let msgWithProof = checkAndGenerateRLNProof(rlnPeer, message).valueOr: - return lighpushErrorResult(OUT_OF_RLN_PROOF, error) + return lighpushErrorResult(LightPushErrorCode.OUT_OF_RLN_PROOF, error) (await wakuRelay.validateMessage(pubSubTopic, msgWithProof)).isOkOr: - return lighpushErrorResult(INVALID_MESSAGE_ERROR, $error) + return lighpushErrorResult(LightPushErrorCode.INVALID_MESSAGE, $error) let publishedResult = await wakuRelay.publish(pubsubTopic, msgWithProof) diff --git a/waku/waku_lightpush/client.nim b/waku/waku_lightpush/client.nim index 2f03b0847..efb330d91 100644 --- a/waku/waku_lightpush/client.nim +++ b/waku/waku_lightpush/client.nim @@ -35,7 +35,8 @@ proc sendPushRequest( let connection = (await wl.peerManager.dialPeer(peer, WakuLightPushCodec)).valueOr: waku_lightpush_v3_errors.inc(labelValues = [dialFailure]) return lighpushErrorResult( - NO_PEERS_TO_RELAY, dialFailure & ": " & $peer & " is not accessible" + LightPushErrorCode.NO_PEERS_TO_RELAY, + dialFailure & ": " & $peer & " is not accessible", ) await connection.writeLP(req.encode().buffer) @@ -44,7 +45,7 @@ proc sendPushRequest( try: buffer = await connection.readLp(DefaultMaxRpcSize.int) except LPStreamRemoteClosedError: - error "Failed to read responose from peer", error = getCurrentExceptionMsg() + error "Failed to read response from peer", error = getCurrentExceptionMsg() return lightpushResultInternalError( "Failed to read response from peer: " & getCurrentExceptionMsg() ) @@ -55,7 +56,7 @@ proc sendPushRequest( return lightpushResultInternalError(decodeRpcFailure) if response.requestId != req.requestId and - response.statusCode != TOO_MANY_REQUESTS.uint32: + response.statusCode != LightPushErrorCode.TOO_MANY_REQUESTS: error "response failure, requestId mismatch", requestId = req.requestId, responseRequestId = response.requestId return lightpushResultInternalError("response failure, requestId mismatch") @@ -105,7 +106,9 @@ proc publishToAny*( let peer = wl.peerManager.selectPeer(WakuLightPushCodec).valueOr: # TODO: check if it is matches the situation - shall we distinguish client side missing peers from server side? - return lighpushErrorResult(NO_PEERS_TO_RELAY, "no suitable remote peers") + return lighpushErrorResult( + LightPushErrorCode.NO_PEERS_TO_RELAY, "no suitable remote peers" + ) let pushRequest = LightpushRequest( requestId: generateRequestId(wl.rng), diff --git a/waku/waku_lightpush/common.nim b/waku/waku_lightpush/common.nim index 4c2984e8f..f2687834e 100644 --- a/waku/waku_lightpush/common.nim +++ b/waku/waku_lightpush/common.nim @@ -5,18 +5,21 @@ import ../waku_core, ./rpc, ../waku_relay/protocol from ../waku_core/codecs import WakuLightPushCodec export WakuLightPushCodec +export LightPushStatusCode -type LightpushStatusCode* = enum - SUCCESS = uint32(200) - BAD_REQUEST = uint32(400) - PAYLOAD_TOO_LARGE = uint32(413) - INVALID_MESSAGE_ERROR = uint32(420) - UNSUPPORTED_PUBSUB_TOPIC = uint32(421) - TOO_MANY_REQUESTS = uint32(429) - INTERNAL_SERVER_ERROR = uint32(500) - SERVICE_NOT_AVAILABLE = uint32(503) - OUT_OF_RLN_PROOF = uint32(504) - NO_PEERS_TO_RELAY = uint32(505) +const LightPushSuccessCode* = (SUCCESS: LightPushStatusCode(200)) + +const LightPushErrorCode* = ( + BAD_REQUEST: LightPushStatusCode(400), + PAYLOAD_TOO_LARGE: LightPushStatusCode(413), + INVALID_MESSAGE: LightPushStatusCode(420), + UNSUPPORTED_PUBSUB_TOPIC: LightPushStatusCode(421), + TOO_MANY_REQUESTS: LightPushStatusCode(429), + INTERNAL_SERVER_ERROR: LightPushStatusCode(500), + SERVICE_NOT_AVAILABLE: LightPushStatusCode(503), + OUT_OF_RLN_PROOF: LightPushStatusCode(504), + NO_PEERS_TO_RELAY: LightPushStatusCode(505), +) type ErrorStatus* = tuple[code: LightpushStatusCode, desc: Option[string]] type WakuLightPushResult* = Result[uint32, ErrorStatus] @@ -28,25 +31,25 @@ type PushMessageHandler* = proc( const TooManyRequestsMessage* = "Request rejected due to too many requests" func isSuccess*(response: LightPushResponse): bool = - return response.statusCode == LightpushStatusCode.SUCCESS.uint32 + return response.statusCode == LightPushSuccessCode.SUCCESS func toPushResult*(response: LightPushResponse): WakuLightPushResult = if isSuccess(response): return ok(response.relayPeerCount.get(0)) else: - return err((response.statusCode.LightpushStatusCode, response.statusDesc)) + return err((response.statusCode, response.statusDesc)) func lightpushSuccessResult*(relayPeerCount: uint32): WakuLightPushResult = return ok(relayPeerCount) func lightpushResultInternalError*(msg: string): WakuLightPushResult = - return err((LightpushStatusCode.INTERNAL_SERVER_ERROR, some(msg))) + return err((LightPushErrorCode.INTERNAL_SERVER_ERROR, some(msg))) func lightpushResultBadRequest*(msg: string): WakuLightPushResult = - return err((LightpushStatusCode.BAD_REQUEST, some(msg))) + return err((LightPushErrorCode.BAD_REQUEST, some(msg))) func lightpushResultServiceUnavailable*(msg: string): WakuLightPushResult = - return err((LightpushStatusCode.SERVICE_NOT_AVAILABLE, some(msg))) + return err((LightPushErrorCode.SERVICE_NOT_AVAILABLE, some(msg))) func lighpushErrorResult*( statusCode: LightpushStatusCode, desc: Option[string] @@ -63,24 +66,22 @@ func mapPubishingErrorToPushResult*( ): WakuLightPushResult = case publishOutcome of NoTopicSpecified: - return err( - (LightpushStatusCode.INVALID_MESSAGE_ERROR, some("Empty topic, skipping publish")) - ) + return + err((LightPushErrorCode.INVALID_MESSAGE, some("Empty topic, skipping publish"))) of DuplicateMessage: - return err( - (LightpushStatusCode.INVALID_MESSAGE_ERROR, some("Dropping already-seen message")) - ) + return + err((LightPushErrorCode.INVALID_MESSAGE, some("Dropping already-seen message"))) of NoPeersToPublish: return err( ( - LightpushStatusCode.NO_PEERS_TO_RELAY, + LightPushErrorCode.NO_PEERS_TO_RELAY, some("No peers for topic, skipping publish"), ) ) of CannotGenerateMessageId: return err( ( - LightpushStatusCode.INTERNAL_SERVER_ERROR, + LightPushErrorCode.INTERNAL_SERVER_ERROR, some("Error generating message id, skipping publish"), ) ) diff --git a/waku/waku_lightpush/protocol.nim b/waku/waku_lightpush/protocol.nim index 45dc7c3c1..955b1ade5 100644 --- a/waku/waku_lightpush/protocol.nim +++ b/waku/waku_lightpush/protocol.nim @@ -36,21 +36,21 @@ proc handleRequest( let msg = "Pubsub topic must be specified when static sharding is enabled" error "lightpush request handling error", error = msg return WakuLightPushResult.err( - (code: LightpushStatusCode.INVALID_MESSAGE_ERROR, desc: some(msg)) + (code: LightPushErrorCode.INVALID_MESSAGE, desc: some(msg)) ) let parsedTopic = NsContentTopic.parse(pushRequest.message.contentTopic).valueOr: let msg = "Invalid content-topic:" & $error error "lightpush request handling error", error = msg return WakuLightPushResult.err( - (code: LightPushStatusCode.INVALID_MESSAGE_ERROR, desc: some(msg)) + (code: LightPushErrorCode.INVALID_MESSAGE, desc: some(msg)) ) wl.autoSharding.get().getShard(parsedTopic).valueOr: let msg = "Auto-sharding error: " & error error "lightpush request handling error", error = msg return WakuLightPushResult.err( - (code: LightPushStatusCode.INTERNAL_SERVER_ERROR, desc: some(msg)) + (code: LightPushErrorCode.INTERNAL_SERVER_ERROR, desc: some(msg)) ) # ensure checking topic will not cause error at gossipsub level @@ -58,7 +58,7 @@ proc handleRequest( let msg = "topic must not be empty" error "lightpush request handling error", error = msg return - WakuLightPushResult.err((code: LightPushStatusCode.BAD_REQUEST, desc: some(msg))) + WakuLightPushResult.err((code: LightPushErrorCode.BAD_REQUEST, desc: some(msg))) waku_lightpush_v3_messages.inc(labelValues = ["PushRequest"]) @@ -78,16 +78,14 @@ proc handleRequest( proc handleRequest*( wl: WakuLightPush, peerId: PeerId, buffer: seq[byte] ): Future[LightPushResponse] {.async.} = - var pushResponse: LightPushResponse - let pushRequest = LightPushRequest.decode(buffer).valueOr: let desc = decodeRpcFailure & ": " & $error error "failed to push message", error = desc - let errorCode = LightPushStatusCode.BAD_REQUEST.uint32 + let errorCode = LightPushErrorCode.BAD_REQUEST waku_lightpush_v3_errors.inc(labelValues = [$errorCode]) return LightPushResponse( requestId: "N/A", # due to decode failure we don't know requestId - statusCode: errorCode.uint32, + statusCode: errorCode, statusDesc: some(desc), ) @@ -96,12 +94,12 @@ proc handleRequest*( waku_lightpush_v3_errors.inc(labelValues = [$error.code]) error "failed to push message", error = desc return LightPushResponse( - requestId: pushRequest.requestId, statusCode: error.code.uint32, statusDesc: desc + requestId: pushRequest.requestId, statusCode: error.code, statusDesc: desc ) return LightPushResponse( requestId: pushRequest.requestId, - statusCode: LightPushStatusCode.SUCCESS.uint32, + statusCode: LightPushSuccessCode.SUCCESS, statusDesc: none[string](), relayPeerCount: some(relayPeerCount), ) @@ -135,7 +133,7 @@ proc initProtocolHandler(wl: WakuLightPush) = ## in reject case as it is comparably too expensive and opens possible ## attack surface requestId: "N/A", - statusCode: LightpushStatusCode.TOO_MANY_REQUESTS.uint32, + statusCode: LightPushErrorCode.TOO_MANY_REQUESTS, statusDesc: some(TooManyRequestsMessage), ) ) diff --git a/waku/waku_lightpush/rpc.nim b/waku/waku_lightpush/rpc.nim index 5a1a6647d..f19563b99 100644 --- a/waku/waku_lightpush/rpc.nim +++ b/waku/waku_lightpush/rpc.nim @@ -3,6 +3,10 @@ import std/options import ../waku_core +type LightPushStatusCode* = distinct uint32 +proc `==`*(a, b: LightPushStatusCode): bool {.borrow.} +proc `$`*(code: LightPushStatusCode): string {.borrow.} + type LightpushRequest* = object requestId*: string @@ -11,6 +15,6 @@ type LightPushResponse* = object requestId*: string - statusCode*: uint32 + statusCode*: LightPushStatusCode statusDesc*: Option[string] relayPeerCount*: Option[uint32] diff --git a/waku/waku_lightpush/rpc_codec.nim b/waku/waku_lightpush/rpc_codec.nim index 53bdda4c0..0a4f934d6 100644 --- a/waku/waku_lightpush/rpc_codec.nim +++ b/waku/waku_lightpush/rpc_codec.nim @@ -43,7 +43,7 @@ proc encode*(rpc: LightPushResponse): ProtoBuffer = var pb = initProtoBuffer() pb.write3(1, rpc.requestId) - pb.write3(10, rpc.statusCode) + pb.write3(10, rpc.statusCode.uint32) pb.write3(11, rpc.statusDesc) pb.write3(12, rpc.relayPeerCount) pb.finish3() @@ -64,7 +64,7 @@ proc decode*(T: type LightPushResponse, buffer: seq[byte]): ProtobufResult[T] = if not ?pb.getField(10, statusCode): return err(ProtobufError.missingRequiredField("status_code")) else: - rpc.statusCode = statusCode + rpc.statusCode = statusCode.LightPushStatusCode var statusDesc: string if not ?pb.getField(11, statusDesc):