[golang] fixed golang api client design mistakes

7 files changed, 220 insertions, 106 deletions

diff --git a/golang/pkg/api/agents.go b/golang/pkg/api/agents.go
index aa8107a..db5b7db 100644
--- a/golang/pkg/api/agents.go
+++ b/golang/pkg/api/agents.go
@@ -1,5 +1,11 @@
 package api
 
+import (
+	"encoding/json"
+	"fmt"
+	"net/url"
+)
+
 type AgentMessage struct {
 	AccountID       string `json:"accountId"`
 	Credits         int    `json:"credits"`
@@ -9,6 +15,18 @@ type AgentMessage struct {
 	Symbol          string `json:"symbol"`
 }
 
-func (c *Client) MyAgent() (APIMessage[AgentMessage, any], error) {
-	return Send[AgentMessage](c, "GET", "/my/agent", nil)
+func (c *Client) MyAgent() (*AgentMessage, error) {
+	uriRef := url.URL{Path: "my/agent"}
+	msg, err := c.Send("GET", &uriRef, nil)
+	if err != nil {
+		return nil, err
+	}
+	if msg.Error != nil {
+		return nil, msg.Error
+	}
+	var response AgentMessage
+	if err := json.Unmarshal(msg.Data, &response); err != nil {
+		return nil, fmt.Errorf("failed to unmarshal agent data: %w", err)
+	}
+	return &response, nil
 }
diff --git a/golang/pkg/api/api.go b/golang/pkg/api/api.go
index 662bb87..7aee2fa 100644
--- a/golang/pkg/api/api.go
+++ b/golang/pkg/api/api.go
@@ -2,86 +2,143 @@ package api
 
 import (
 	"bytes"
+	"container/heap"
 	"encoding/json"
+	"fmt"
 	"io"
 	"log/slog"
 	"net/http"
+	"net/url"
 	"time"
 )
 
-type Error[T any] struct {
-	Code    int    `json:"code"`
-	Data    T      `json:"data"`
-	Message string `json:"message"`
+type APIError struct {
+	Code    int             `json:"code"`
+	Data    json.RawMessage `json:"data"`
+	Message string          `json:"message"`
 }
 
-type APIMessage[T any, E any] struct {
-	Data  T        `json:"data"`
-	Error Error[E] `json:"error"`
+func (e *APIError) Error() string {
+	return fmt.Sprintf("unhandled APIError code %d, message \"%s\", data: %s", e.Code, e.Message, string(e.Data))
+}
+
+type APIMessage struct {
+	Data  json.RawMessage `json:"data"`
+	Error *APIError       `json:"error"`
 	//meta
 }
 
+type Request struct {
+	index    int
+	priority int
+
+	method          string
+	uri             *url.URL
+	payload         any
+	responseChannel chan *Response
+}
+
 type Response struct {
-	Response []byte
-	Err      error
+	Message *APIMessage
+	Err     error
 }
 
-func Send[T any](c *Client, method, path string, payload any) (message APIMessage[T, any], err error) {
-	resp := make(chan *Response)
-	c.channel <- &Request{
-		method:   method,
-		path:     path,
-		payload:  payload,
-		priority: 10,
-		resp:     resp,
+func (c *Client) Send(method string, uriRef *url.URL, payload any) (*APIMessage, error) {
+	responseChannel := make(chan *Response)
+	c.requestsChannel <- &Request{
+		method:          method,
+		payload:         payload,
+		priority:        10,
+		responseChannel: responseChannel,
+		uri:             c.baseURI.ResolveReference(uriRef),
 	}
-	res := <-resp
-	if res.Err != nil {
-		return message, res.Err
+	res := <-responseChannel
+	return res.Message, res.Err
+}
+
+func queueProcessor(client *Client) {
+	var (
+		req *Request
+		ok  bool
+	)
+	for {
+		// The queue is empty so we do this blocking call
+		select {
+		case <-client.ctx.Done():
+			return
+		case req, ok = <-client.requestsChannel:
+			if !ok {
+				return
+			}
+			heap.Push(client.pq, req)
+		}
+		// we enqueue all values read from the channel and process the queue's
+		// contents until empty. We keep reading the channel as long as this
+		// emptying goes on
+		for {
+			select {
+			case <-client.ctx.Done():
+				return
+			case req, ok = <-client.requestsChannel:
+				if !ok {
+					return
+				}
+				heap.Push(client.pq, req)
+			default:
+				if client.pq.Len() == 0 {
+					break
+				}
+				// we process one
+				if req, ok = heap.Pop(client.pq).(*Request); !ok {
+					panic("queueProcessor got something other than a Request on its channel")
+				}
+				msg, err := client.sendOne(req.method, req.uri, req.payload)
+				req.responseChannel <- &Response{
+					Message: msg,
+					Err:     err,
+				}
+			}
+		}
 	}
-	err = json.Unmarshal(res.Response, &message)
-	return message, err
 }
 
-func (c *Client) sendOne(method, path string, payload any) (body []byte, err error) {
-	slog.Debug("Request", "method", method, "path", path, "payload", payload)
-	var req *http.Request
+func (c *Client) sendOne(method string, uri *url.URL, payload any) (*APIMessage, error) {
+	slog.Debug("request", "method", method, "path", uri.Path, "payload", payload)
+	var payloadReader io.Reader
 	if payload != nil {
-		body, err = json.Marshal(payload)
-		if err == nil {
-			req, err = http.NewRequest(method, c.baseURL+path, bytes.NewBuffer(body))
+		if body, err := json.Marshal(payload); err != nil {
+			return nil, fmt.Errorf("failed to marshal payload: %w", err)
 		} else {
-			return nil, err
+			payloadReader = bytes.NewReader(body)
 		}
-	} else {
-		req, err = http.NewRequest(method, c.baseURL+path, nil)
 	}
+
+	req, err := http.NewRequestWithContext(c.ctx, method, uri.String(), payloadReader)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("failed to create request: %w", err)
 	}
 	req.Header = *c.headers
-	req = req.WithContext(c.ctx)
 
 	resp, err := c.httpClient.Do(req)
 	if err != nil {
-		slog.Error("sendOne Do", "method", method, "path", path, "error", err)
-		return nil, err
+		return nil, fmt.Errorf("failed to do request: %w", err)
 	}
-	defer func() {
-		if e := resp.Body.Close(); err == nil {
-			err = e
-		}
-	}()
-	if body, err = io.ReadAll(resp.Body); err != nil {
-		slog.Error("sendOne ReadAll", "method", method, "path", path, "error", err)
-		return nil, err
+	defer resp.Body.Close()
+	body, err := io.ReadAll(resp.Body)
+	if err != nil {
+		return nil, fmt.Errorf("failed to read response body: %w", err)
+	}
+
+	var msg APIMessage
+	if err = json.Unmarshal(body, &msg); err != nil {
+		return nil, fmt.Errorf("failed to unmarshal response body: %w", err)
 	}
-	slog.Debug("Response", "body", string(body))
+	slog.Debug("response", "code", resp.StatusCode, "message", msg)
 	switch resp.StatusCode {
 	case 429:
-		e := decode429(body)
-		time.Sleep(time.Duration(e.Error.Data.RetryAfter * float64(time.Second)))
-		return c.sendOne(method, path, payload)
+		e := decodeRateLimitError(msg.Error.Data)
+		time.Sleep(e.RetryAfter.Duration() * time.Second)
+		return c.sendOne(method, uri, payload)
 	}
-	return body, nil
+	return &msg, nil
 }
diff --git a/golang/pkg/api/client.go b/golang/pkg/api/client.go
index 70f3e68..2ea555e 100644
--- a/golang/pkg/api/client.go
+++ b/golang/pkg/api/client.go
@@ -4,25 +4,30 @@ import (
 	"container/heap"
 	"context"
 	"net/http"
+	"net/url"
 	"time"
 )
 
 type Client struct {
-	baseURL    string
-	channel    chan *Request
-	ctx        context.Context
-	headers    *http.Header
-	httpClient *http.Client
-	pq         *PriorityQueue
+	baseURI         *url.URL
+	requestsChannel chan *Request
+	ctx             context.Context
+	headers         *http.Header
+	httpClient      *http.Client
+	pq              *PriorityQueue
 }
 
 func NewClient(ctx context.Context) *Client {
+	baseURI, err := url.Parse("https://api.spacetraders.io/v2/")
+	if err != nil {
+		panic("baseURI failed to parse")
+	}
 	pq := make(PriorityQueue, 0)
 	heap.Init(&pq)
 	client := &Client{
-		baseURL: "https://api.spacetraders.io/v2",
-		channel: make(chan *Request),
-		ctx:     ctx,
+		baseURI:         baseURI,
+		requestsChannel: make(chan *Request),
+		ctx:             ctx,
 		headers: &http.Header{
 			"Content-Type": {"application/json"},
 		},
@@ -36,40 +41,9 @@ func NewClient(ctx context.Context) *Client {
 }
 
 func (c *Client) Close() {
-	close(c.channel)
+	close(c.requestsChannel)
 }
 
 func (c *Client) SetToken(token string) {
 	c.headers.Set("Authorization", "Bearer "+token)
 }
-
-func queueProcessor(client *Client) {
-	var ok bool
-	for {
-		// The queue is empty so we do this blocking call
-		req := <-client.channel
-		heap.Push(client.pq, req)
-		// we enqueue all values read from the channel and process the queue's
-		// contents until empty. We keep reading the channel as long as this
-		// emptying goes on
-		for {
-			select {
-			case req = <-client.channel:
-				heap.Push(client.pq, req)
-			default:
-				if client.pq.Len() == 0 {
-					break
-				}
-				// we process one
-				if req, ok = heap.Pop(client.pq).(*Request); !ok {
-					panic("queueProcessor got something other than a Request on its channel")
-				}
-				response, err := client.sendOne(req.method, req.path, req.payload)
-				req.resp <- &Response{
-					Response: response,
-					Err:      err,
-				}
-			}
-		}
-	}
-}
diff --git a/golang/pkg/api/duration.go b/golang/pkg/api/duration.go
new file mode 100644
index 0000000..c2ba1fd
--- /dev/null
+++ b/golang/pkg/api/duration.go
@@ -0,0 +1,38 @@
+package api
+
+import (
+	"encoding/json"
+	"errors"
+	"time"
+)
+
+type Duration time.Duration
+
+func (d *Duration) Duration() time.Duration {
+	return time.Duration(*d)
+}
+
+func (d *Duration) MarshalJSON() ([]byte, error) {
+	return json.Marshal(time.Duration(*d).String())
+}
+
+func (d *Duration) UnmarshalJSON(b []byte) error {
+	var v interface{}
+	if err := json.Unmarshal(b, &v); err != nil {
+		return err
+	}
+	switch value := v.(type) {
+	case float64:
+		*d = Duration(time.Duration(value))
+		return nil
+	case string:
+		tmp, err := time.ParseDuration(value)
+		if err != nil {
+			return err
+		}
+		*d = Duration(tmp)
+		return nil
+	default:
+		return errors.New("invalid duration")
+	}
+}
diff --git a/golang/pkg/api/errors.go b/golang/pkg/api/errors.go
index d39a205..2e36f16 100644
--- a/golang/pkg/api/errors.go
+++ b/golang/pkg/api/errors.go
@@ -6,18 +6,37 @@ import (
 	"time"
 )
 
+// ----- 429 --------------------------------------------------------------------
 type RateLimitError struct {
 	LimitType      string    `json:"type"`
-	RetryAfter     float64   `json:"retryAfter"`
+	RetryAfter     Duration  `json:"retryAfter"`
 	LimitBurst     int       `json:"limitBurst"`
 	LimitPerSecond int       `json:"limitPerSecond"`
 	Remaining      int       `json:"remaining"`
 	Reset          time.Time `json:"reset"`
 }
 
-func decode429(msg []byte) (e APIMessage[any, RateLimitError]) {
+func decodeRateLimitError(msg json.RawMessage) RateLimitError {
+	var e RateLimitError
 	if err := json.Unmarshal(msg, &e); err != nil {
-		panic(fmt.Sprintf("Failed to decode419: %+v", err))
+		panic(fmt.Errorf("Failed to decode iapi error code 429 RateLimitError: %v, %w", msg, err))
+	}
+	return e
+}
+
+// ----- 4214 -------------------------------------------------------------------
+type ShipInTransitError struct {
+	Arrival           time.Time `json:"arrival"`
+	DepartureSymbol   string    `json:"departureSymbol"`
+	DepartureTime     time.Time `json:"departureTime"`
+	DestinationSymbol string    `json:"destinationSymbol"`
+	SecondsToArrival  Duration  `json:"secondsToArrival"`
+}
+
+func decodeShipInTransitError(msg json.RawMessage) ShipInTransitError {
+	var e ShipInTransitError
+	if err := json.Unmarshal(msg, &e); err != nil {
+		panic(fmt.Errorf("Failed to decode api error code 4214 ShipInTransitError: %v, %w", msg, err))
 	}
 	return e
 }
diff --git a/golang/pkg/api/priority_queue.go b/golang/pkg/api/priority_queue.go
index 077c8f7..08e3258 100644
--- a/golang/pkg/api/priority_queue.go
+++ b/golang/pkg/api/priority_queue.go
@@ -1,15 +1,5 @@
 package api
 
-type Request struct {
-	index    int
-	priority int
-
-	method  string
-	path    string
-	payload any
-	resp    chan *Response
-}
-
 type PriorityQueue []*Request
 
 func (pq PriorityQueue) Len() int {
diff --git a/golang/pkg/api/register.go b/golang/pkg/api/register.go
index 4f95e2f..4f45cd1 100644
--- a/golang/pkg/api/register.go
+++ b/golang/pkg/api/register.go
@@ -1,5 +1,11 @@
 package api
 
+import (
+	"encoding/json"
+	"fmt"
+	"net/url"
+)
+
 type RegisterMessage struct {
 	//agent
 	//contract
@@ -8,13 +14,25 @@ type RegisterMessage struct {
 	Token string `json:"token"`
 }
 
-func (c *Client) Register(faction, symbol string) (APIMessage[RegisterMessage, any], error) {
+func (c *Client) Register(faction, symbol string) (*RegisterMessage, error) {
 	type RegisterRequest struct {
 		Faction string `json:"faction"`
 		Symbol  string `json:"symbol"`
 	}
-	return Send[RegisterMessage](c, "POST", "/register", RegisterRequest{
+	uriRef := url.URL{Path: "register"}
+	msg, err := c.Send("POST", &uriRef, RegisterRequest{
 		Faction: faction,
 		Symbol:  symbol,
 	})
+	if err != nil {
+		return nil, err
+	}
+	if msg.Error != nil {
+		return nil, msg.Error
+	}
+	var response RegisterMessage
+	if err := json.Unmarshal(msg.Data, &response); err != nil {
+		return nil, fmt.Errorf("failed to unmarshal register data: %w", err)
+	}
+	return &response, nil
 }