RPC Refactor (#182)

* Return a ChiaRPCError if the request succeeds but the RPC returns an error. Always return the response object even if there's an error in the response since there is still useful information sometimes

* Add generic Do() helper to make adding rpc endpoints simpler. Convert full node to use Do()

* Refactor remaining services to use the Generic Do helper to reduce boilerplate

* Check for err != nil before checking the resp to avoid nil pointer issues

* Fix incorrect endpoint

pkg/httpclient/httpclient.go

@@ -171,7 +171,7 @@ func (c *HTTPClient) NewRequest(service rpcinterface.ServiceType, rpcEndpoint rp
 }
 
 // Do sends an RPC request and returns the RPC response.
-func (c *HTTPClient) Do(req *rpcinterface.Request, v interface{}) (*http.Response, error) {
+func (c *HTTPClient) Do(req *rpcinterface.Request, v rpcinterface.IResponse) (*http.Response, error) {
 	client, err := c.httpClientForService(req.Service)
 	if err != nil {
 		return nil, err

pkg/publichttpclient/httpclient.go

@@ -132,7 +132,7 @@ func (c *HTTPClient) NewRequest(service rpcinterface.ServiceType, rpcEndpoint rp
 }
 
 // Do sends an RPC request and returns the RPC response.
-func (c *HTTPClient) Do(req *rpcinterface.Request, v interface{}) (*http.Response, error) {
+func (c *HTTPClient) Do(req *rpcinterface.Request, v rpcinterface.IResponse) (*http.Response, error) {
 	client, err := c.httpClientForService(req.Service)
 	if err != nil {
 		return nil, err

pkg/rpc/client.go

@@ -1,7 +1,9 @@
 package rpc
 
 import (
+	"log/slog"
 	"net/http"
+	"net/url"
 
 	"github.com/google/uuid"
 
@@ -87,8 +89,26 @@ func (c *Client) NewRequest(service rpcinterface.ServiceType, rpcEndpoint rpcint
 }
 
 // Do is a helper that wraps the activeClient's Do method
-func (c *Client) Do(req *rpcinterface.Request, v interface{}) (*http.Response, error) {
-	return c.activeClient.Do(req, v)
+func (c *Client) Do(req *rpcinterface.Request, v rpcinterface.IResponse) (*http.Response, error) {
+	resp, err := c.activeClient.Do(req, v)
+	if err != nil {
+		return resp, err
+	}
+	if !v.IsSuccessful() {
+		return resp, &rpcinterface.ChiaRPCError{Message: v.GetRPCError()}
+	}
+	return resp, nil
+}
+
+// Do Helper to create and send a new request for a given service and retain the proper types
+func Do[R rpcinterface.IResponse](service rpcinterface.Service, endpoint rpcinterface.Endpoint, opts any, v R) (R, *http.Response, error) {
+	req, err := service.NewRequest(endpoint, opts)
+	if err != nil {
+		return v, nil, err
+	}
+
+	resp, err := service.GetClient().Do(req, v)
+	return v, resp, err
 }
 
 // Close calls the close method on the active client
@@ -98,6 +118,16 @@ func (c *Client) Close() error {
 
 // The following has a bunch of methods that are currently only used for the websocket implementation
 
+// SetBaseURL satisfies the Client interface
+func (c *Client) SetBaseURL(url *url.URL) error {
+	return c.activeClient.SetBaseURL(url)
+}
+
+// SetLogHandler satisfies the client interface
+func (c *Client) SetLogHandler(handler slog.Handler) {
+	c.activeClient.SetLogHandler(handler)
+}
+
 // SubscribeSelf subscribes to responses to requests from this service
 // This is currently only useful for websocket mode
 func (c *Client) SubscribeSelf() error {

pkg/rpc/crawler.go

@@ -19,64 +19,30 @@ func (s *CrawlerService) NewRequest(rpcEndpoint rpcinterface.Endpoint, opt inter
 	return s.client.NewRequest(rpcinterface.ServiceCrawler, rpcEndpoint, opt)
 }
 
-// Do is just a shortcut to the client's Do method
-func (s *CrawlerService) Do(req *rpcinterface.Request, v interface{}) (*http.Response, error) {
-	return s.client.Do(req, v)
+// GetClient returns the active client for the service
+func (s *CrawlerService) GetClient() rpcinterface.Client {
+	return s.client
 }
 
 // GetNetworkInfo gets the network name and prefix from the full node
 func (s *CrawlerService) GetNetworkInfo(opts *GetNetworkInfoOptions) (*GetNetworkInfoResponse, *http.Response, error) {
-	request, err := s.NewRequest("get_network_info", opts)
-	if err != nil {
-		return nil, nil, err
-	}
-
-	r := &GetNetworkInfoResponse{}
-
-	resp, err := s.Do(request, r)
-	if err != nil {
-		return nil, resp, err
-	}
-
-	return r, resp, nil
+	return Do(s, "get_network_info", opts, &GetNetworkInfoResponse{})
 }
 
 // GetVersion returns the application version for the service
 func (s *CrawlerService) GetVersion(opts *GetVersionOptions) (*GetVersionResponse, *http.Response, error) {
-	request, err := s.NewRequest("get_version", opts)
-	if err != nil {
-		return nil, nil, err
-	}
-
-	r := &GetVersionResponse{}
-	resp, err := s.Do(request, r)
-	if err != nil {
-		return nil, resp, err
-	}
-
-	return r, resp, nil
+	return Do(s, "get_version", opts, &GetVersionResponse{})
 }
 
 // GetPeerCountsResponse Response for get_get_peer_counts on crawler
 type GetPeerCountsResponse struct {
-	Response
+	rpcinterface.Response
 	PeerCounts mo.Option[types.CrawlerPeerCounts] `json:"peer_counts"`
 }
 
 // GetPeerCounts crawler rpc -> get_peer_counts
 func (s *CrawlerService) GetPeerCounts() (*GetPeerCountsResponse, *http.Response, error) {
-	request, err := s.NewRequest("get_peer_counts", nil)
-	if err != nil {
-		return nil, nil, err
-	}
-
-	r := &GetPeerCountsResponse{}
-	resp, err := s.Do(request, r)
-	if err != nil {
-		return nil, resp, err
-	}
-
-	return r, resp, nil
+	return Do(s, "get_peer_counts", nil, &GetPeerCountsResponse{})
 }
 
 // GetIPsAfterTimestampOptions Options for the get_ips_after_timestamp RPC call
@@ -88,23 +54,12 @@ type GetIPsAfterTimestampOptions struct {
 
 // GetIPsAfterTimestampResponse Response for get_ips_after_timestamp
 type GetIPsAfterTimestampResponse struct {
-	Response
+	rpcinterface.Response
 	IPs   mo.Option[[]string] `json:"ips"`
 	Total mo.Option[int]      `json:"total"`
 }
 
 // GetIPsAfterTimestamp Returns IP addresses seen by the network after a particular timestamp
 func (s *CrawlerService) GetIPsAfterTimestamp(opts *GetIPsAfterTimestampOptions) (*GetIPsAfterTimestampResponse, *http.Response, error) {
-	request, err := s.NewRequest("get_ips_after_timestamp", opts)
-	if err != nil {
-		return nil, nil, err
-	}
-
-	r := &GetIPsAfterTimestampResponse{}
-	resp, err := s.Do(request, r)
-	if err != nil {
-		return nil, resp, err
-	}
-
-	return r, resp, nil
+	return Do(s, "get_ips_after_timestamp", opts, &GetIPsAfterTimestampResponse{})
 }