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][DEVX-548]Add Retry for (401) Invalid Token Error #950

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

ajimae
Copy link
Contributor

@ajimae ajimae commented Feb 26, 2025

Summary

Add retry for (401) invalid token

Completed Tasks

  • add a 401 status check in auth middleware
  • move token processor to an authProcessor function
  • refactor code to remove duplicate logic
  • add integration tests

Background 📚

In rare cases, a token may become invalid on the server side while still being present on the client, either in the Authorization header or the tokenCache.

This PR addresses this edge case by ensuring that the client application always has a valid token.

@ajimae ajimae requested a review from a team as a code owner February 26, 2025 22:11
Copy link

changeset-bot bot commented Feb 26, 2025

🦋 Changeset detected

Latest commit: c25a7a5

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@commercetools/ts-client Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link

codecov bot commented Feb 27, 2025

Codecov Report

Attention: Patch coverage is 98.00000% with 1 line in your changes missing coverage. Please review.

Project coverage is 97.19%. Comparing base (0cfacd7) to head (c25a7a5).

Files with missing lines Patch % Lines
...ddleware/auth-middleware/auth-request-processor.ts 97.14% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #950      +/-   ##
==========================================
+ Coverage   96.73%   97.19%   +0.45%     
==========================================
  Files         112      113       +1     
  Lines        2145     2104      -41     
  Branches      633      605      -28     
==========================================
- Hits         2075     2045      -30     
+ Misses         63       52      -11     
  Partials        7        7              
Flag Coverage Δ
-coverage 97.19% <98.00%> (+0.45%) ⬆️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@barbara79
Copy link
Contributor

Good Job! 👍
Did you consider to update the documentation on the github pages and on the official documentation?

@ajimae
Copy link
Contributor Author

ajimae commented Mar 5, 2025

Good Job! 👍 Did you consider to update the documentation on the github pages and on the official documentation?

There is no API change hence there is nothing to update, besides, the github pages is automated and will be publish on the next release.

Copy link
Contributor

@lojzatran lojzatran left a comment

Choose a reason for hiding this comment

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

Great job! I just some small comments 👍

ajimae and others added 9 commits March 5, 2025 23:34
- add a 401 status check in auth middleware
- move token processor to an authProcessor function
- refactor code to remove duplicate logic
- add integration tests
- use correct token expiry time
- add more tests to improve coverage
- add more tests to improve coverage
- add more tests to improve coverage
- add more tests to improve coverage
- add more tests to improve coverage
- rename function from _tokenCache to tokenStore
@ajimae ajimae force-pushed the feat/add-retry-for-specific-401-errors branch from 7ee31ff to f637028 Compare March 5, 2025 22:35
- use concise and meaningful function names
- parallelize Promise function for simulating conconcurrency
@ajimae ajimae force-pushed the feat/add-retry-for-specific-401-errors branch from f637028 to c25a7a5 Compare March 5, 2025 22:39
@barbara79
Copy link
Contributor

Good Job! 👍 Did you consider to update the documentation on the github pages and on the official documentation?

There is no API change hence there is nothing to update, besides, the github pages is automated and will be publish on the next release.

Yes, I know that there isn't any API change, what I mean is that if this method could be used by the customer, please remember to update the official documentation.
I checked the flow better and probably this is not the case.

@ajimae ajimae requested review from lojzatran and jenschude March 10, 2025 09:09
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

Successfully merging this pull request may close these issues.

4 participants