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

feat: exponential retries for axiosWrapper #980

Merged
merged 1 commit into from
Nov 8, 2023
Merged

feat: exponential retries for axiosWrapper #980

merged 1 commit into from
Nov 8, 2023

Conversation

dkwon17
Copy link
Contributor

@dkwon17 dkwon17 commented Nov 7, 2023

What does this PR do?

Tweaks the axiosWrapper retries functionality to have exponential retries with a base of 2, and an initial delay of 500ms.

As a result, with 3 retries, the first retry has a delay of 500ms, second has a delay of 1000ms and third with 2000ms.

What issues does this PR fix or reference?

eclipse-che/che#22652

Is it tested? How?

To simulate 401 errors and test this PR, checkout the PR branch and apply this patch:

diff --git a/packages/dashboard-backend/src/routes/api/helpers/getToken.ts b/packages/dashboard-backend/src/routes/api/helpers/getToken.ts
index 59e8d76f..cd44ec45 100644
--- a/packages/dashboard-backend/src/routes/api/helpers/getToken.ts
+++ b/packages/dashboard-backend/src/routes/api/helpers/getToken.ts
@@ -18,6 +18,9 @@ const authorizationBearerPrefix = /^Bearer /;
 
 export function getToken(request: FastifyRequest): string {
   const authorization = request.headers?.authorization;
+  if (Math.random() <= 0.9) {
+    throw createFastifyError('FST_UNAUTHORIZED', 'Bearer Token Authorization is required', 401);
+  }
   if (!authorization || !authorizationBearerPrefix.test(authorization)) {
     throw createFastifyError('FST_UNAUTHORIZED', 'Bearer Token Authorization is required', 401);
   }

and build the image.

Next follow these steps:

  1. Deploy Che and set the che-dashboard image to the new image.

  2. Go to the dashboard and open your web browser web developer tools

  3. Refresh the page

  4. The console should print statements like the following:
    Screenshot from 2023-11-07 14-47-01

  5. Confirm that the retries delay is 500ms for the first retry, 1000ms for the second, and 1500ms for the third

Release Notes

Docs PR

@che-bot
Copy link
Contributor

che-bot commented Nov 7, 2023

Click here to review and test in web IDE: Contribute

@dkwon17 dkwon17 requested a review from tolusha November 7, 2023 20:05
await delay(this.retryDelay);
// Retry the request after a exponential delay.
const exp = this.retryCount - retry;
const expDelay = this.initialDelay * this.base ** exp;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the same as

 this.initialDelay * (this.base ** exp);

but prettier complains about the extra brackets

Copy link

codecov bot commented Nov 7, 2023

Codecov Report

Merging #980 (55cfa9d) into main (2476cab) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main     #980   +/-   ##
=======================================
  Coverage   85.15%   85.15%           
=======================================
  Files         380      380           
  Lines       39186    39189    +3     
  Branches     2522     2522           
=======================================
+ Hits        33370    33373    +3     
  Misses       5789     5789           
  Partials       27       27           
Flag Coverage Δ
unittests 85.15% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
...rontend/src/services/axios-wrapper/axiosWrapper.ts 98.30% <100.00%> (+0.04%) ⬆️

Copy link

openshift-ci bot commented Nov 8, 2023

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: dkwon17, ibuziuk, tolusha

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Copy link

github-actions bot commented Nov 8, 2023

Docker image build succeeded: quay.io/eclipse/che-dashboard:pr-980

@ibuziuk ibuziuk merged commit cd4440e into main Nov 8, 2023
13 checks passed
@ibuziuk ibuziuk deleted the tweak-retries branch November 8, 2023 14:28
@devstudio-release
Copy link

Build 3.11 :: dashboard_3.x/389: Console, Changes, Git Data

@devstudio-release
Copy link

Build 3.10 :: dashboard_3.10/17: Console, Changes, Git Data

@devstudio-release
Copy link

@devstudio-release
Copy link

@devstudio-release
Copy link

@devstudio-release
Copy link

@devstudio-release
Copy link

@devstudio-release
Copy link

@devstudio-release
Copy link

@devstudio-release
Copy link

Build 3.11 :: dashboard_3.x/389: SUCCESS

Upstream sync done; /DS_CI/sync-to-downstream_3.x/5205 triggered

@devstudio-release
Copy link

@devstudio-release
Copy link

Build 3.10 :: dashboard_3.10/17: SUCCESS

Upstream sync done; /DS_CI/sync-to-downstream_3.10/112 triggered

@devstudio-release
Copy link

@devstudio-release
Copy link

@devstudio-release
Copy link

@devstudio-release
Copy link

@devstudio-release
Copy link

Build 3.11 :: update-digests_3.x/4839: UNSTABLE

No new images detected: nothing to do!

@devstudio-release
Copy link

@devstudio-release
Copy link

@devstudio-release
Copy link

Build 3.10 :: copyIIBsToQuay/2112: Console, Changes, Git Data

@devstudio-release
Copy link

Build 3.10 :: sync-to-downstream_3.10/113: SUCCESS

Build container: devspaces-operator-bundle synced; /DS_CI/get-sources-rhpkg-container-build_3.10/114 triggered; /job/DS_CI/job/dsc_3.10 triggered;

@devstudio-release
Copy link

Build 3.10 :: operator-bundle_3.10/40: SUCCESS

Upstream sync done; /DS_CI/sync-to-downstream_3.10/113 triggered

@devstudio-release
Copy link

Build 3.10 :: update-digests_3.10/139: SUCCESS

Detected new images: rebuild operator-bundle
* dashboard; /DS_CI/operator-bundle_3.10/40 triggered

@devstudio-release
Copy link

Build 3.10 :: dsc_3.10/32: Console, Changes, Git Data

@devstudio-release
Copy link

Build 3.10 :: dsc_3.10/32: SUCCESS

3.10.0-CI

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants