Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

feat: add refreshTokenHandler option #7237

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

Draft
WojtekTheWebDev wants to merge 4 commits into main
base: main
Choose a base branch
Loading
from feat/add-refresh-token-handler

Conversation

@WojtekTheWebDev
Copy link
Collaborator

@WojtekTheWebDev WojtekTheWebDev commented Aug 21, 2024

πŸ”— Linked issue

❓ Type of change

  • πŸ“– Documentation (updates to the documentation, readme or JSDoc annotations)
  • 🐞 Bug fix (a non-breaking change that fixes an issue)
  • πŸ‘Œ Enhancement (improving an existing functionality like performance)
  • ✨ New feature (a non-breaking change that adds functionality)
  • 🧹 Chore (updates to the build process or auxiliary tools and libraries)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

πŸ“š Description

Adds a new optional property refreshTokenHandler to the middlewareModule

πŸ“ Checklist

  • I have linked an issue or discussion.
  • I have updated the documentation accordingly.

Copy link

changeset-bot bot commented Aug 21, 2024
edited
Loading

πŸ¦‹ Changeset detected

Latest commit: 6810360

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

This PR includes changesets to release 1 package
Name Type
@vue-storefront/sdk 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
Collaborator

@bartoszherba bartoszherba left a comment

Choose a reason for hiding this comment

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

Few comments qdded

[ADDED] new option to the `middlewareModule` - `refreshTokenHandler`.
This special handler can be used to handle 401 errors and refresh the token.
It is called before the generic `errorHandler`.
By default, it thrown an error which is being caught by the `errorHandler` and rethrown.
Copy link
Collaborator

@bartoszherba bartoszherba Aug 22, 2024

Choose a reason for hiding this comment

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

I think that some small diagram (even ascii) with the sequence will be useful here. Or maybe we can somehow rephrase this sentence, I think I would not understand it without my knowledge about how it works. Something like an explanation that by default we are not able to predict the mechanism of token refresh because of differences in different services therefore we do not provide any default behavior, we simply push the error to the errorHandler without any further actions.

methodName,
url,
params,
config,
Copy link
Collaborator

@bartoszherba bartoszherba Aug 22, 2024

Choose a reason for hiding this comment

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

Is this a module config? Or what kind of config? Will I have some docs in TS? If not maybe lets explain the injected object here?

@WojtekTheWebDev WojtekTheWebDev marked this pull request as draft August 22, 2024 19:47
Copy link
Collaborator Author

Converted to draft as I'm still not convinced that it is the best solution.

Copy link

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bartoszherba bartoszherba bartoszherba approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

AltStyle γ«γ‚ˆγ£γ¦ε€‰ζ›γ•γ‚ŒγŸγƒšγƒΌγ‚Έ (->γ‚ͺγƒͺγ‚ΈγƒŠγƒ«) /