Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add GraphQL endpoint #228

Merged
merged 8 commits into from
Aug 5, 2023
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion carrier.go
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ type ShippingRate struct {
TotalPrice decimal.Decimal `json:"total_price"`

// Whether the customer must provide a phone number at checkout.
PhoneRequired bool `json:phone_required,omitempty"`
PhoneRequired bool `json:"phone_required,omitempty"`

// The earliest delivery date for the displayed rate.
MinDeliveryDate *time.Time `json:"min_delivery_date"` // "2013-04-12 14:48:45 -0400"
Expand Down
3 changes: 3 additions & 0 deletions goshopify.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ type App struct {
type RateLimitInfo struct {
RequestCount int
BucketSize int
GraphQLCost *GraphQLCost
RetryAfterSeconds float64
}

Expand Down Expand Up @@ -122,6 +123,7 @@ type Client struct {
CarrierService CarrierServiceService
Payouts PayoutsService
GiftCard GiftCardService
GraphQL GraphQLService
}

// A general response error that follows a similar layout to Shopify's response
Expand Down Expand Up @@ -303,6 +305,7 @@ func NewClient(app App, shopName, token string, opts ...Option) *Client {
c.CarrierService = &CarrierServiceOp{client: c}
c.Payouts = &PayoutsServiceOp{client: c}
c.GiftCard = &GiftCardServiceOp{client: c}
c.GraphQL = &GraphQLServiceOp{client: c}

// apply any options
for _, opt := range opts {
Expand Down
149 changes: 149 additions & 0 deletions graphql.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,149 @@
package goshopify

import (
"math"
"time"
)

// GraphQLService is an interface to interact with the graphql endpoint
// of the Shopify API
// See https://shopify.dev/docs/admin-api/graphql/reference
type GraphQLService interface {
Query(string, interface{}, interface{}) (int, error)
}

// GraphQLServiceOp handles communication with the graphql endpoint of
// the Shopify API.
type GraphQLServiceOp struct {
client *Client
}

type graphQLResponse struct {
Data interface{} `json:"data"`
Errors []graphQLError `json:"errors"`
Extensions *graphQLExtension `json:"extensions"`
}

type graphQLExtension struct {
Cost GraphQLCost `json:"cost"`
}

// GraphQLCost represents the cost of the graphql query
type GraphQLCost struct {
RequestedQueryCost int `json:"requestedQueryCost"`
ActualQueryCost *int `json:"actualQueryCost"`
ThrottleStatus GraphQLThrottleStatus `json:"throttleStatus"`
}

// GraphQLThrottleStatus represents the status of the shop's rate limit points
type GraphQLThrottleStatus struct {
MaximumAvailable float64 `json:"maximumAvailable"`
CurrentlyAvailable float64 `json:"currentlyAvailable"`
RestoreRate float64 `json:"restoreRate"`
}

type graphQLError struct {
Message string `json:"message"`
Extensions *graphQLErrorExtensions `json:"extensions"`
Locations []graphQLErrorLocation `json:"locations"`
}

type graphQLErrorExtensions struct {
Code string
Documentation string
}

const (
graphQLErrorCodeThrottled = "THROTTLED"
)

type graphQLErrorLocation struct {
Line int `json:"line"`
Column int `json:"column"`
}

// Query creates a graphql query against the Shopify API
// the "data" portion of the response is unmarshalled into resp
// Returns the number of attempts required to perform the request
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's worth mentioning that Query() will retry throttled requests but not e..g any other type of 500 error.

Copy link
Author

@weirdian2k3 weirdian2k3 Aug 4, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To keep the concept of retry more defined, I added this comment to the WithRetry method

// WithRetry sets the number of times a request will be retried if a rate limit or service unavailable error is returned.
// Rate limiting can be either REST API limits or GraphQL Cost limits

func (s *GraphQLServiceOp) Query(q string, vars, resp interface{}) (int, error) {
data := struct {
Query string `json:"query"`
Variables interface{} `json:"variables"`
}{
Query: q,
Variables: vars,
}

attempts := 0

for {
gr := graphQLResponse{
Data: resp,
}

err := s.client.Post("graphql.json", data, &gr)
// internal attempts count towards outer total
attempts += s.client.attempts
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I looked at the code again and I think this is a problem.
This is not threadsafe when multiple goroutines use the same client and make requests at the same time.
The state attempts part of the client is shared across requests and can't be relied on for truthfully telling the number of attempts made.
This should be something simple to show in a test (have the mock responder wait a sec for each request and respond with "throttled", then kick off 10 requests, all with the same client, then check attempts, should be the combined number of requests or so).

I think I'd want to remove attempts altogether and for instance have an alternate/secondary version of Post that returns the number of attempts.

This might be getting a bit much for this PR, to simplify you could, as first step, replace the attempts+=... with attempts += 1 andthen we can figure out later if this needs improving.

An alternate option would be to make the retry in doGetHeaders() smarter and possibly make it aware of GraphQL. But again, that's a bigger effort and I'd be fine with the simplified +=1

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I went with the += 1 because I'm trying to ship something that depends on this, but I'll try to loop back and do a more proper management in another PR


var ra float64
weirdian2k3 marked this conversation as resolved.
Show resolved Hide resolved

if gr.Extensions != nil {
ra = gr.Extensions.Cost.RetryAfterSeconds()
s.client.RateLimits.GraphQLCost = &gr.Extensions.Cost
s.client.RateLimits.RetryAfterSeconds = ra
}

if len(gr.Errors) > 0 {
re := ResponseError{Status: 200}
var doRetry bool

for _, err := range gr.Errors {
if err.Extensions != nil && err.Extensions.Code == graphQLErrorCodeThrottled {
if attempts >= s.client.retries {
return attempts, RateLimitError{
RetryAfter: int(math.Ceil(ra)),
ResponseError: ResponseError{
Status: 200,
Message: err.Message,
},
}
}

// only need to retry graphql throttled retries
doRetry = true
}

re.Errors = append(re.Errors, err.Message)
}

if doRetry {
wait := time.Duration(math.Ceil(ra)) * time.Second
s.client.log.Debugf("rate limited waiting %s", wait.String())
time.Sleep(wait)
continue
}

err = re
}

return attempts, err
}
}

// RetryAfterSeconds returns the estimated retry after seconds based on
// the requested query cost and throttle status
func (c GraphQLCost) RetryAfterSeconds() float64 {
var diff float64

if c.ActualQueryCost != nil {
diff = c.ThrottleStatus.CurrentlyAvailable - float64(*c.ActualQueryCost)
} else {
diff = c.ThrottleStatus.CurrentlyAvailable - float64(c.RequestedQueryCost)
}

if diff < 0 {
return -diff / c.ThrottleStatus.RestoreRate
oliver006 marked this conversation as resolved.
Show resolved Hide resolved
}

return 0
}
Loading