Skip to content

Fix using auto_paging_iter() with expand: [...] #1434

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

Merged
merged 2 commits into from
Dec 17, 2024
Merged

Conversation

xavdid-stripe
Copy link
Member

Why

When you called auto_paging_iter() on a list that you fetched with expand, the server would give a 400 error. Ultimately, this was caused by the SDK making a malformed request with similar-but-different expand values.

When you turn a response into a StripeObject, we store the request url that created that response. That url includes any query params. So, given this code:

lines = stripe.Invoice.upcoming_lines(
    limit=3, 
    customer='cus_ROQYYBI1BraV53',
    expand=['data.price.product']
)

hits the url: /v1/invoices/upcoming/lines?customer=cus_ROQYYBI1BraV53&expand%5B%5D=data.price.product

In addition to the URL, Python's StripeObject also stores the params used to create the object. On subsequent requests with the same object (e.g. when paging through a list) the SDK adds the original params to paging related ones (such as starting_after) to the original url to make the request.

Because the server returned the params and Python still has the params, we end up with duplicates in the url:

https://api.stripe.com/v1/invoices/upcoming/lines?customer=cus_ROQYYBI1BraV53&expand%5B%5D=data.price.product&limit=3&customer=cus_ROQYYBI1BraV53&expand[0]=data.price.product&starting_after=il_tmp_1QVdh7JoUivz182DmwLpUU1y

Not only is customer duplicated, there's both expand[] and expand[0] keys. The latter is what's causing the issue. Clients can send expand[0]=a&expand[1]=b or expand[]=a&expand[]=b, but can't mix.

The server sends extend[] in its url response and python adds indexes, so the clobbering was the root of the issue.

To fix, I parsed out the existing query params from a url and merged them with params, which avoids duplicates. I also merged expand values just in case someone is expanding something new on the next_page() call that wasn't expanded originally.

This approach is a little kludgy, but does work and fix the bug. I'm open to other suggestions though!

What

  • when making a GET or DELETE request with additional params, deduplicate params.
  • add test

Testing

import stripe
stripe.api_key = "sk_test_..."
# set up customer with more than one page of invoice items
customer = stripe.Customer.create()
for i in range(5):
    stripe.InvoiceItem.create(
        customer=customer.id,
        price="price_1PVu24JoUivz182D067aXjgy"
    )
    print(f"Created invoice item {i}")
print(f"Done creating invoice items for {customer.id}, now listing them")
# try to iterate over all invoice items
lines = stripe.Invoice.upcoming_lines(
    limit=3, 
    customer=customer.id,
    expand=['data.price.product']
)
for line in lines.auto_paging_iter(): # errors after 3
   print(f"{line.id}")

fixes RUN_DEVSDK-1427

(
"%s?foo=bar&foo=bar" % stripe.api_base,
Copy link
Member Author

Choose a reason for hiding this comment

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

this section is mostly removing % formatting, but I also removed the duplicate foo=bar since that's not really a supported pattern.

@xavdid-stripe xavdid-stripe merged commit ef9d5b0 into master Dec 17, 2024
15 checks passed
@xavdid-stripe xavdid-stripe deleted the RUN_DEVSDK-1427 branch December 17, 2024 22:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants