From 0d00bf9fd242d3b4f9f3d6caaf5a61f8a3627d7f Mon Sep 17 00:00:00 2001 From: Julien Dessaux Date: Tue, 28 May 2024 13:13:13 +0200 Subject: [golang] fixed golang api client design mistakes --- golang/pkg/api/agents.go | 22 +++++- golang/pkg/api/api.go | 153 +++++++++++++++++++++++++++------------ golang/pkg/api/client.go | 56 ++++---------- golang/pkg/api/duration.go | 38 ++++++++++ golang/pkg/api/errors.go | 25 ++++++- golang/pkg/api/priority_queue.go | 10 --- golang/pkg/api/register.go | 22 +++++- 7 files changed, 220 insertions(+), 106 deletions(-) create mode 100644 golang/pkg/api/duration.go (limited to 'golang/pkg/api') 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 } -- cgit v1.2.3