contentful / contentful-sdk-core

Core modules for the Contentful JS SDKs
MIT License
22 stars 20 forks source link

The rate-limit interceptor removes all URI parameters on the first retry #513

Closed spzSource closed 8 hours ago

spzSource commented 2 weeks ago

This change, introduced a few months ago within #483, broke the contentful-migration tooling, leading to a significant production issue for one of Contentful's premium customers. For more details, see the following issue: contentful/contentful-migration#1384.

It essentially alters the original URI and removes all existing parameters after the initial request fails during the first retry.

Tagging @t-col as the author of the change.

t-col commented 3 days ago

Thanks for raising the issue and providing a fix @spzSource

The original hack that you reference here was meant to address this issue: https://github.com/contentful/contentful.js/issues/2246. If you're curious, you can follow the steps described in that issue with the versions that were applicable at the time, and you'll find the opposite issue that you're addressing here (which was that query params were being repeatedly appended to the URL on each retry).

The actual bug was indeed a problem introduced by axios (in https://github.com/axios/axios/pull/6371), and it appears it was later fixed in https://github.com/axios/axios/pull/6515.

spzSource commented 9 hours ago

Hi @t-col,

Thanks for the clarification. Hopefully, the test I introduced will help catch these types of issues if any regressions occur.

contentful-automation[bot] commented 8 hours ago

:tada: This issue has been resolved in version 8.3.2 :tada:

The release is available on:

Your semantic-release bot :package::rocket: