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

autoPagingInterator() fails if the last collection item is deleted #1264

Open
snez opened this issue Apr 13, 2022 · 4 comments
Open

autoPagingInterator() fails if the last collection item is deleted #1264

snez opened this issue Apr 13, 2022 · 4 comments

Comments

@snez
Copy link

snez commented Apr 13, 2022

When the autoPagingInterator() is used with deletes, only 1 page can be processed, because the last item of the current collection has been deleted, therefore the pager cannot find it to build the 2nd page.

For example (PHP)

        $products = $stripe->products->all(['limit' => 100]);
        foreach ($products->autoPagingIterator() as $product)
        {
            $product->delete();
        }

The above will crash with No such product: 'prod_LIvx6FPSkqMLyL' because the last item in the collection no longer exists.

This can be a problem in a number of cases, i.e.

  • Looping through and deleting duplicated saved payment methods based on fingerprint data.
  • Test suites where Stripe test data must be cleared before the test suite starts.
  • Synchronizing local products with Stripe products in cron jobs.
  • etc
@dcr-stripe
Copy link
Contributor

Thanks for the report @snez .

Unfortunately this is the result of mutating your objects while iterating.

autoPagingIterator will automatically fetch the next page of data when necessary using starting_after cursor. For some resource types, pointing starting_after to a resource that has been deleted fails as it cannot resume pagination.

We intentionally don't pre-fetch all of the data at the initial iterator creation since this could fetch a substantial amount of data that the user might not need.

We'll look internally to see if there's something we could improve here. Potentially we could try to scan back through the data page to check what has and has not been deleted but this is very precarious (things might have been deleted in flight), assumes that all resources behave this way (they don't - customer deletion is different), and relies on the fact that we currently mutate model objects as the result of API calls.

In the interim I would suggest aggregating all the list of items to delete and doing this deletion after you are done iterating.

Hope that helps explain what's going on here!

@snez
Copy link
Author

snez commented Apr 13, 2022

@dcr-stripe there are various possible solutions

  • You could fetch 101 results so that pagination starts the 2nd page at 101
  • You could use the last timestamp instead of the last object for the pagination
  • You could mark the object as deleted internally in Stripe without actually deleting it, and purge it later
  • You could fetch the 2nd page at the 99th iteration instead of the 100th
  • etc

@richardm-stripe
Copy link
Contributor

Just as an update, there is no fix for this immediately forthcoming. There's nothing we can do in stripe-php itself to address the problem satisfactorily, the fix would have to involve a (breaking) redesign of how pagination works in the Stripe API. I'm keeping this ticket open as I believe this is an important usability problem that should be visible, but did want to set expectations.

@ZacharyDuBois
Copy link

If the Stripe ID are lexicographically sortable (like ULIDs), which it appears they are, there is really no reason to validate the previous/last ID when paging. If that last ID was deleted, the next item still won't change and there is no reason to validate the provided last ID.

IE A < B is always true even if I delete A.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants