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

Retry with given config leads to CanceledError #1232

Closed
1 of 2 tasks
trochol opened this issue Oct 25, 2024 · 6 comments
Closed
1 of 2 tasks

Retry with given config leads to CanceledError #1232

trochol opened this issue Oct 25, 2024 · 6 comments

Comments

@trochol
Copy link

trochol commented Oct 25, 2024

Did you read the migration guide?

  • I have read the whole migration guide

Is there an existing issue that is already proposing this?

  • I have searched the existing issues

Potential Commit/PR that introduced the regression

commit dcbc102

Versions

3.0.3 -> 3.1.0

Describe the regression

With the bugfix for the following issue a new bug was introduced:
#1217
Before the given config was cloned with the spread operator.
In the bugfix the config was taken as is and the AxiosRequestConfig.cancelToken was added if there was none in the config.

Now, if you are using the rxjs retry and pass a config without CancelToken, the given config will be extended with the default CancelToken.
And if the request fails the retry subscribes again to observable with the original config, which then has a fulfilled CancelToken of the first try.

Minimum reproduction code

https://github.com/trochol/axios-retry/tree/main

Expected behavior

The retry operator should be able tosubscribe to the observable and a new CancelToken shall be added to the config.

Other

No response

@kamilmysliwiec
Copy link
Member

Would you like to create a PR for this issue?

@trochol
Copy link
Author

trochol commented Oct 28, 2024

In my opinion I would revert the commit of PR #1217.
The CancelToken includes a promise, which leads to the cancel, when fulfilled.
The caller can pass an own promise and resolve it whenever he needs to.
So why should it be possible to manipulate the config, after it has been passed to axios?

@trochol
Copy link
Author

trochol commented Oct 28, 2024

Sorry, now I saw the main problem of PR #1217.
It's not the possibility to manually add the CancelToken later.
The problem is that the config copy was extended but never passed to axios.
Then I can create a PR to fix my problem and keep the other issue in mind.
But I will create a copy of the config again, so manually setting the CancelToken after creating the observable won't be possible.

@kamilmysliwiec
Copy link
Member

Thank you @trochol

kamilmysliwiec added a commit that referenced this issue Oct 29, 2024
fix: clone axios args and req config #1232
@trochol
Copy link
Author

trochol commented Oct 30, 2024

Thanks for merging the PR @kamilmysliwiec!

@trochol trochol closed this as completed Oct 30, 2024
@tobiasschweizer
Copy link

Thanks for this!

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

No branches or pull requests

3 participants