fix: return appropriate error code for filter subscribe ping (#975)

cmd/waku/server/rest/filter.go

@@ -78,6 +78,27 @@ func NewFilterService(node *node.WakuNode, m *chi.Mux, cacheCapacity int, log *z
 	return s
 }
 
+func convertFilterErrorToHttpStatus(err error) (int, string) {
+	code := http.StatusInternalServerError
+	statusDesc := "ping request failed"
+
+	filterErrorCode := filter.ExtractCodeFromFilterError(err.Error())
+	switch filterErrorCode {
+	case 404:
+		code = http.StatusNotFound
+		statusDesc = "peer has no subscription"
+	case 300:
+	case 400:
+		code = http.StatusBadRequest
+		statusDesc = "bad request format"
+	case 504:
+		code = http.StatusGatewayTimeout
+	case 503:
+		code = http.StatusServiceUnavailable
+	}
+	return code, statusDesc
+}
+
 // 400 for bad requestId
 // 404 when request failed or no suitable peers
 // 200 when ping successful
@@ -99,10 +120,14 @@ func (s *FilterService) ping(w http.ResponseWriter, req *http.Request) {
 
 	if err := s.node.FilterLightnode().Ping(req.Context(), peerId, filter.WithPingRequestId([]byte(requestID))); err != nil {
 		s.log.Error("ping request failed", zap.Error(err))
+
+		code, statusDesc := convertFilterErrorToHttpStatus(err)
+
 		writeResponse(w, &filterSubscriptionResponse{
 			RequestID:  requestID,
-			StatusDesc: "ping request failed",
+			StatusDesc: statusDesc,
-		}, http.StatusServiceUnavailable)
+		}, code)
+
 		return
 	}
 
@@ -153,10 +178,14 @@ func (s *FilterService) subscribe(w http.ResponseWriter, req *http.Request) {
 
 	if err != nil {
 		s.log.Error("subscription failed", zap.Error(err))
+		code := filter.ExtractCodeFromFilterError(err.Error())
+		if code == -1 {
+			code = http.StatusBadRequest
+		}
 		writeResponse(w, filterSubscriptionResponse{
 			RequestID:  message.RequestID,
 			StatusDesc: "subscription failed",
-		}, http.StatusServiceUnavailable)
+		}, code)
 		return
 	}
 
@@ -195,6 +224,12 @@ func (s *FilterService) unsubscribe(w http.ResponseWriter, req *http.Request) {
 
 	if err != nil {
 		s.log.Error("unsubscribe failed", zap.Error(err))
+		if result == nil {
+			writeResponse(w, filterSubscriptionResponse{
+				RequestID:  message.RequestID,
+				StatusDesc: err.Error(),
+			}, http.StatusBadRequest)
+		}
 		writeResponse(w, filterSubscriptionResponse{
 			RequestID:  message.RequestID,
 			StatusDesc: err.Error(),

cmd/waku/server/rest/filter_test.go

@@ -79,9 +79,9 @@ func TestFilterPingFailure(t *testing.T) {
 	router.ServeHTTP(rr, req)
 	checkJSON(t, filterSubscriptionResponse{
 		RequestID:  requestID,
-		StatusDesc: "ping request failed",
+		StatusDesc: "peer has no subscription",
 	}, getFilterResponse(t, rr.Body))
-	require.Equal(t, http.StatusServiceUnavailable, rr.Code)
+	require.Equal(t, http.StatusNotFound, rr.Code)
 }
 
 // create a filter subscription to the peer and try peer that peer
@@ -230,9 +230,9 @@ func TestFilterAllUnsubscribe(t *testing.T) {
 	router.ServeHTTP(rr, req)
 	checkJSON(t, filterSubscriptionResponse{
 		RequestID:  requestID,
-		StatusDesc: "ping request failed",
+		StatusDesc: "peer has no subscription",
 	}, getFilterResponse(t, rr.Body))
-	require.Equal(t, http.StatusServiceUnavailable, rr.Code)
+	require.Equal(t, http.StatusNotFound, rr.Code)
 }
 
 func checkJSON(t *testing.T, expected, actual interface{}) {

waku/v2/protocol/filter/common.go

@@ -22,6 +22,18 @@ func NewFilterError(code int, message string) FilterError {
 	}
 }
 
+const errorStringFmt = "%d - %s"
+
 func (e *FilterError) Error() string {
-	return fmt.Sprintf("%d - %s", e.Code, e.Message)
+	return fmt.Sprintf(errorStringFmt, e.Code, e.Message)
+}
+
+func ExtractCodeFromFilterError(fErr string) int {
+	code := 0
+	var message string
+	_, err := fmt.Sscanf(fErr, errorStringFmt, &code, &message)
+	if err != nil {
+		return -1
+	}
+	return code
 }