stripe / stripe-go

Go library for the Stripe API.
https://stripe.com
MIT License
2.15k stars 459 forks source link

Unexpected Automatic Invoice Item to Invoice Association #1343

Closed naftulikay closed 3 years ago

naftulikay commented 3 years ago

I'm observing some extremely strange behavior from the Stripe SDK in Golang.

Background

$ go version
go version go1.16.6 linux/amd64
$ grep stripe/stripe-go go.sum
github.com/stripe/stripe-go/v72 v72.64.1 h1:LsT6QVC8xF4X/Kp8xsNYqvubE3vuXn4/dhOFLJSmRRQ=
github.com/stripe/stripe-go/v72 v72.64.1/go.mod h1:QwqJQtduHubZht9mek5sds9CtQcKFdsykV9ZepRWwo0=

I have done go clean, go mod download, and go mod verify and then go build to compile again.

Code

I'm doing the following, and perhaps it can be done more elegantly, but this code does actually work:

  1. Look up a Stripe product by its product name (a constraint of the situation I'm in).
  2. Look up a Stripe product's price by its value (again, a project constraint for now).
  3. Create a Stripe invoice without any line items.
  4. Create a Stripe invoice item without associating it with the invoice.
func CreateInvoice(customerID string, metaObject StripeMetadataObject, productName string, productPrice decimal.Decimal, currency string) (*stripe.Invoice, error) {
    currency = strings.ToUpper(currency)

    // get the stripe product
    stripeProduct := getStripeProductByName(productName)

    if stripeProduct == nil {
        return nil, fmt.Errorf("unable to find stripe product with name '%s'", productName)
    }

    // find the product's price with the given value
    stripePrice := getStripeProductPriceByValue(stripeProduct.ID, productPrice, currency)

    if stripePrice == nil {
        return nil, fmt.Errorf("unable to find price `%s/%s` for product %s", productPrice, currency, stripeProduct.ID)
    }

    // create the stripe invoice
    stripeInvoice, err := createStripeInvoice(customerID, metaObject)

    if err != nil {
        return nil, fmt.Errorf("unable to create stripe invoice: %s", err)
    }

    // create the stripe invoice item
    stripeInvoiceItem, err := createStripeInvoiceItem(customerID, stripePrice.ID, 1)

    if err != nil {
        return nil, fmt.Errorf("unable to create stripe invoice item: %s", err)
    }

    log.Printf("invoice: %s, invoice_item: %s", stripeInvoice.ID, stripeInvoiceItem.ID)

    // FIXME unknown black magic at work here
    // I have no idea how it works, but invoiceitem.New updates stripeInvoice in spite of not having a reference to it.
    // Does it use some kind of goroutine specific framework for associating items?
    return stripeInvoice, nil
}

func createStripeInvoiceItem(customerID string, priceID string, quantity uint) (*stripe.InvoiceItem, error) {
    params := stripe.InvoiceItemParams{
        Customer: stripe.String(customerID),
        Price:    stripe.String(priceID),
        Quantity: stripe.Int64(int64(quantity)),
    }

    return invoiceitem.New(&params)
}

func createStripeInvoice(customerID string, meta StripeMetadataObject) (*stripe.Invoice, error) {
    params := stripe.InvoiceParams{
        AutoAdvance: stripe.Bool(true),
        Customer:    stripe.String(customerID),
        Params: stripe.Params{
            Metadata: meta.StripeMetadata(),
        },
    }

    return invoice.New(&params)
}

// getStripeProductByName Get a Stripe product object by an exact match on its name.
func getStripeProductByName(name string) *stripe.Product {
    // FIXME this is rather inefficient and I don't know a way to filter the search to a single name
    active := true
    iter := product.List(&stripe.ProductListParams{Active: &active})

    for iter.Next() {
        value := iter.Product()

        if value.Name == name {
            return value
        }
    }

    return nil
}

// getStripeProductPriceByValue Given a product ID, find a price by its unit amount.
func getStripeProductPriceByValue(productID string, cost decimal.Decimal, currency string) *stripe.Price {
    active := true
    iter := price.List(&stripe.PriceListParams{
        Active:  &active,
        Product: &productID,
    })

    for iter.Next() {
        value := iter.Price()

        if strings.ToLower(string(value.Currency)) == strings.ToLower(currency) && value.UnitAmount == cost.IntPart() {
            return value
        }
    }

    return nil
}

Expected Behavior

My expectation was that I was going to need to either create the invoice and its InvoiceItems in one fell swoop, but this does not work and the Stripe SDK throws an error saying that invoice_items is not a recognized parameter.

This may be very well be a bug in the Stripe SDK; attempt to create a stripe.InvoiceParams with InvoiceItems with a single stripe.InvoiceUpcomingInvoiceItemParams, passing in the Stripe price, etc. and it will fail when creating the invoice with the given message.

Again, what I'm doing is:

  1. Create an invoice without any line items.
  2. Create an invoice line item without explicitly associating it with the invoice; stripe.InvoiceItemParams does not have its Invoice field set to any value, it is nil.

I expect this to fail because I'm creating an orphaned line item with it having no way of knowing to be associated with an invoice.

Actual Behavior

Somehow, even though I'm not associating the line item with the invoice, the Stripe Go SDK infers what I'm doing and automatically associates the line item with the invoice without any of my own doing. Additionally, the stripe.Invoice object in memory has the stripe.InvoiceItem automatically added to it, again: without any of my own doing.

Questions

Does the Stripe Go SDK keep a reference tracker of all stripe.* objects in memory? Does it look them up and update them when it sees them updated in an API call? Does the Stripe SDK use some kind of goroutine-local information to infer what I'm doing based on the sequence of my actions?

Summary

The Stripe Go SDK, after creation of a stripe.Invoice, automatically associates stripe.InvoiceItems created after it with the stripe.Invoice created beforehand, using some sleight-of-hand or Go runtime black magic. This is unexpected, and when I attempt to do it the "right" way (i.e. by associating invoice items explicitly or by including them in the stripe.InvoiceParams when I create invoices), the API rejects my requests.

naftulikay commented 3 years ago

When I attempt line item creation within the new invoice request like so:

func createStripeInvoice(customerID string, priceID string, meta StripeMetadataObject) (*stripe.Invoice, error) {
    params := stripe.InvoiceParams{
        AutoAdvance: stripe.Bool(true),
        Customer:    stripe.String(customerID),
        InvoiceItems: []*stripe.InvoiceUpcomingInvoiceItemParams{
            {
                Price:    stripe.String(priceID),
                Quantity: stripe.Int64(1),
            },
        },
        Params: stripe.Params{
            Metadata: meta.StripeMetadata(),
        },
    }

    return invoice.New(&params)
}

The error message I receive is:

[ERROR] Request error from Stripe (status 400): {"code":"parameter_unknown","doc_url":"https://stripe.com/docs/error-codes/parameter-unknown","status":400,"message":"Received unknown parameter: invoice_items","param":"invoice_items","request_id":"req_03w2NwvqM7T6ro","type":"invalid_request_error"}
naftulikay commented 3 years ago

I'm really at a loss for what I'm observing; I may be observing some really strange memory unsafety. The same commit hash of my code which was working before is now no longer working:

[ERROR] Request error from Stripe (status 400): {"code":"invoice_no_customer_line_items","doc_url":"https://stripe.com/docs/error-codes/invoice-no-customer-line-items","status":400,"message":"Nothing to invoice for customer","param":"customer","request_id":"req_JVO7GVugqmJdN6","type":"invalid_request_error"}
alinalebron commented 3 years ago

@naftulikay link for the answers you seeeeekkkk

https://stripe.com/docs/invoicing/integration >> only look at the section labeled 5 onwards Creating an invoice Server-side

It notes:

If you need to invoice a customer, create an invoice item by passing the customer ID, currency, and price. You can then create an invoice for your customer and the invoice item will be added automatically

naftulikay commented 3 years ago

Interesting; as mentioned by @alinalebron here is the Go code in the documentation:

// Set your secret key. Remember to switch to your live secret key in production.
// See your keys here: https://dashboard.stripe.com/apikeys
stripe.Key = "sk_test_51HfAeCAYOXBYcifHc0klKJyt9ltvwNFKjDabtpHXhv1jzMGzvyxFK0Ph2TkdrByyceTyC2fYhzkD7IUfnV59Qpqc00s3vNuI8R"

params := &stripe.InvoiceItemParams{
  Customer: stripe.String("cus_4fdAW5ftNQow1a"),
  Price: stripe.String("price_CBb6IXqvTLXp3f"),
}
ii, _ := invoiceitem.New(params)

invParams := &stripe.InvoiceParams{
  Customer: stripe.String("cus_4fdAW5ftNQow1a"),
  AutoAdvance: stripe.Bool(true),
}
in, _ := invoice.New(invParams)

Apparently, the behavior I'm witnessing is the expected behavior. In the example, the invoice item and invoice which are created have no explicit references to each other. Either the Stripe Go SDK is doing some weird stuff or the Stripe API itself has some kind of "memory" and contextualizes seemingly unrelated requests with one another.

remi-stripe commented 3 years ago

@naftulikay Thanks for the detailed write up! The question here is really more an integration question where you need help with your code/understanding our product and not really a bug in the stripe-go library. As @alinalebron mentioned, this is covered in details in our documentation!

In the API, you can't create a "blank invoice". You have to have at least one invoice item pending on the customer associated with that customer to create it first. Once it's created, it pulls all pending invoice items from the customer onto that invoice automatically. After that, new invoice items will stay pending since the invoice has already been created. If you want those new invoice items to be tied to the invoice, you're expected to pass the existing invoice id in_123 in the Invoice parameter. This is useful so that you can have multiple separate invoices in parallel before finalization and we know which invoice you're adding the item to.

Overall, the behaviour you were expecting does not exist in our API today, though it's something we've discussed adding in the past and likely will in the future.

I'm going to close this issue out since there's no bug with stripe-go or our API and it works as expected. I do recommend joining our Discord server so that you can chat about your code with developers and Stripe engineers if you have more questions: https://stripe.com/go/developer-chat

naftulikay commented 3 years ago

@remi-stripe thank you for responding.

This does seem to have a race condition though. Imagine the following scenario:

I know that API implementation details are secret, but are you implementing a per customer global mutex to prevent this from happening?

Maybe this is rare enough that it likely won't happen, but debugging this would be a nightmare.

IIRC when I tried to create a invoice with explicit references to the item, the API reported an error.

remi-stripe commented 3 years ago

@naftulikay That's totally fair but it's more by design than anything else today and it's been designed this way for as long as the API has existed. I agree there's a race condition but it's something implicit.

The flow you are looking for is definitely something we want to build in the future, it's just not as simple as it looks, especially rolling it out without breaking all the merchants that rely on today's behaviour. It's in our plans, and something we've started scoping before, but no short term plans to change this behaviour today (though I do agree it can be surprising/confusing as is).

naftulikay commented 3 years ago

@remi-stripe thanks for your reply, just trying to make sure I understand things properly :)