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

Don't delete session on 401 error #1844

Merged
merged 2 commits into from
May 6, 2024
Merged

Don't delete session on 401 error #1844

merged 2 commits into from
May 6, 2024

Conversation

zzooeeyy
Copy link
Contributor

@zzooeeyy zzooeeyy commented May 6, 2024

Addresses this concern: #1795 (comment)

What this PR does

Not deleting the session when 401 error has occurred in token exchange retry.

Checklist

Before submitting the PR, please consider if any of the following are needed:

  • Update CHANGELOG.md if the changes would impact users
  • Update README.md, if appropriate.
  • Update any relevant pages in /docs, if necessary
  • For security fixes, the Disclosure Policy must be followed.

@zzooeeyy zzooeeyy marked this pull request as ready for review May 6, 2024 16:56
@zzooeeyy zzooeeyy requested a review from a team as a code owner May 6, 2024 16:56
@zzooeeyy zzooeeyy changed the title Don't delete shop session on 401 error Don't delete session on 401 error May 6, 2024
Copy link
Contributor

@paulomarg paulomarg left a comment

Choose a reason for hiding this comment

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

Is this behaviour present in remix too? Might be something we want to change there too, as folks make a good point about being overzealous in cleaning up data.

@zzooeeyy zzooeeyy merged commit 6f845b0 into main May 6, 2024
7 checks passed
@zzooeeyy zzooeeyy deleted the dont-delete-session branch May 6, 2024 17:08
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.

None yet

2 participants