Skip to content

Commit

Permalink
fix: do not remove query parameters on retry (#514)
Browse files Browse the repository at this point in the history
Fixes #513.
This is a draft fix intended to start the discussion.

I'm not sure if it's okay to remove the existing hack introduced in
#483. Before making a decision, I’d like to learn more about the
duplicate request parameters problem and how I can reproduce it in a
test.

@t-col probably you could give me some hints?
  • Loading branch information
spzSource authored Sep 16, 2024
1 parent 0afab0f commit fbc7975
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 8 deletions.
8 changes: 0 additions & 8 deletions src/rate-limit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,14 +81,6 @@ export default function rateLimit(instance: AxiosInstance, maxRetry = 5): void {
delete config.httpAgent
delete config.httpsAgent

/**
* Hack to mitigate (likely) bug introduced in axios v1.7.0 where `url`,
* when using the default `xhr` adapter, is a fully qualified URL (instead of a path),
* which somehow causes the request params to be repeatedly appended
* to the final request URL upon each retry.
*/
config.url = config.url.split('?')[0]

return delay(wait).then(() => instance(config))
}
return Promise.reject(error)
Expand Down
19 changes: 19 additions & 0 deletions test/unit/rate-limit-test.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -169,3 +169,22 @@ it('Rejects errors with strange status codes', async () => {
expect(logHandlerStub).toBeCalledTimes(0)
}
})

it('Preserves URI query parameters between retries', async () => {
const { client } = setup()
const uri = '/entries?content_type=B&fields.id=1'

mock.onGet(uri).replyOnce(429, 'Rate Limit')
mock.onGet(uri).replyOnce(429, 'Rate Limit')
mock.onGet(uri).replyOnce(200, 'Success')

expect.assertions(5)

const response = await client.get(uri)
expect(response.status).toEqual(200)

expect(mock.history.get.length).toBe(3)
for (const request of mock.history.get) {
expect(request.url).toEqual(uri)
}
})

0 comments on commit fbc7975

Please sign in to comment.