feat: cap maximum number of retries at 25
#343
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #343 +/- ##
==========================================
+ Coverage 56.86% 57.00% +0.13%
==========================================
Files 16 16
Lines 728 735 +7
Branches 113 114 +1
==========================================
+ Hits 414 419 +5
- Misses 303 305 +2
Partials 11 11 ☔ View full report in Codecov by Sentry. |
| @@ -136,6 +136,9 @@ await ofetch("http://google.com/404", { | |||
| }); | |||
| ``` | |||
|
|
|||
| > [!NOTE] | |||
There was a problem hiding this comment.
I think we can use a normal paragraph since no user action is involved...
There was a problem hiding this comment.
Good point!
| // Retry | ||
| if (context.options.retry !== false && !isAbort) { | ||
| let retries; | ||
| if (typeof context.options.retry === "number") { | ||
| retries = context.options.retry; | ||
| if (retries > MAX_RETRIES) { |
There was a problem hiding this comment.
What about introducing new maxRetries option to allow setting the default?
There was a problem hiding this comment.
I wonder if this defeats the purpose of this PR. My understanding was to include a sensible default for maximum retries. Instead of a maxRetries option, retries basically does the same, right?
If you dislike the idea of an internal maximum, which I completely get, please close this PR. 🙂
There was a problem hiding this comment.
Well it could be a configurable global in fetch factory so basically it is second cap but also configurable if someone/framework/util needs too :)
|
I don't think these changes are useful to most users. There are much more interesting PRs at hand to elevate ofetch to the next level, such as #357 for local/global interceptors. So I'm closing this PR. All the best to ofetch! Love it. 🙌 |
🔗 Linked issue
Related:
❓ Type of change
📚 Description
This PR adds a sanity default value for the configurable
retriesoption.This enhancement is suggested by @pi0 following the security issue detected by @OhB00. In the Nuxt module API Party the fetch options were passed to a server route, which consumed the fetch options as-is. This enabled attackers to force a
retryvalue of whatever.It might be best to set a maximum number of retries. Not only for security reasons, but also for developers as a safety measures.
📝 Checklist