[PB-5908]: add global retry with rate-limit notifications for SDK clients#1866
Open
[PB-5908]: add global retry with rate-limit notifications for SDK clients#1866
Conversation
Deploying drive-web with
|
| Latest commit: |
50d9d22
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://49361b8f.drive-web.pages.dev |
| Branch Preview URL: | https://feature-global-retry.drive-web.pages.dev |
terrerox
commented
Feb 24, 2026
| - uses: actions/setup-node@v4 | ||
| with: | ||
| node-version: '20.11.1' | ||
| node-version: '20.19.0' |
Contributor
Author
There was a problem hiding this comment.
Bumped from 20.11.1 to 20.19.0 to fix failing Playwright CI job.
@noble/hashes@2.0.1 (transitive dep of @internxt/sdk@1.15.0) requires Node >= 20.19.0,
which caused yarn install to fail with "Found incompatible module".
CandelR
reviewed
Feb 26, 2026
CandelR
reviewed
Mar 3, 2026
CandelR
previously approved these changes
Mar 3, 2026
Upgrade @internxt/sdk to 1.15.0 and introduce retry strategies for HTTP requests. Silent retries (2 attempts) are enabled globally, while Storage and Share clients use a user-facing notification strategy (5 attempts) with a cooldown-based warning toast. Adds hasElapsed utility to date.service and i18n translations for the rate-limit toast in all supported languages
…rable max retries
…d prevent multiple calls
…ge and share clients
…way upload conflicts The global HttpClient.enableGlobalRetry was applying retry to all SDK clients including the gateway Network client, which broke multi-step uploads (start → upload parts → finish) by retrying individual steps independently. Moving retryOptions into ApiSecurity enables retry only for drive API clients (Storage, Share, Users, etc.) while leaving the gateway client retry-free — uploads already have operation-level retry.
terrerox
force-pushed
the
feature/global-retry
branch
from
6f1179d to
c0f0d74
Compare
Previously, per-client retryOptions only covered Storage/Share clients in the main thread, leaving gateway startUpload/finishUpload calls in Web Workers without 429 handling. This restores HttpClient.enableGlobalRetry in both the main thread and upload worker context.
…ent gateway upload conflicts" This reverts commit c0f0d74.
|
| import { t } from 'i18next'; | ||
| import { retryStrategies, NotifyUserCallback } from './retryStrategies'; | ||
|
|
||
| const RETRY_TOAST_DURATION_MS = 60000; |
Contributor
There was a problem hiding this comment.
Maybe we should move all constants in src/app/core/constants.ts so that they are all in one place. @CandelR do we have a file with constants specific to sdk or is the one in core the only one?
sg-gs
approved these changes
Mar 10, 2026
| return Math.max(0, Math.ceil(diffInDays)); | ||
| }; | ||
|
|
||
| const hasElapsed = (since: Dayjs, amount: number, unit: dayjs.ManipulateType): boolean => |
Contributor
There was a problem hiding this comment.
Do we need a function for this? It's called in only one place and it's a one-line function
Contributor
There was a problem hiding this comment.
For me its better that way:
- Readability: hasElapsed(since, 5, 'minute') reads closer to plain English than dayjs().diff(since, 'minute') >= 5, which requires 'knowing' the dayjs API
- Testability: a named function is easier to unit test in isolation
- Future reuse: "called in only one place now" doesn't mean it won't be reused later 🤔
- Abstraction: it hides the dayjs dependency, so if you ever swap date libraries, there's one place to change
wdyt? @TamaraFinogina
| @@ -0,0 +1,23 @@ | |||
| import { RetryOptions } from '@internxt/sdk/dist/shared'; | |||
|
|
|||
| export const SILENT_MAX_RETRIES = 2; | |||
Contributor
There was a problem hiding this comment.
Same with those constants
larryrider
approved these changes
Mar 10, 2026
TamaraFinogina
approved these changes
Mar 10, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Upgrade @internxt/sdk to 1.15.0 and introduce retry strategies for HTTP requests. Silent retries (2 attempts) are enabled globally, while Storage and Share clients use a user-facing notification strategy (5 attempts) with a cooldown-based warning toast. Adds hasElapsed utility to date.service and i18n translations for the rate-limit toast in all supported languages
Related Issues
Related Pull Requests
Checklist
Testing Process
Additional Notes