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 propagate "not exist" error if trying to erase a session matchi… #252

Merged
merged 3 commits into from Aug 17, 2023
Merged

Conversation

mariusor
Copy link
Contributor

@mariusor mariusor commented Dec 29, 2021

Summary of Changes

Don't consider "oserror.ErrNotExist" as a failure when trying to erase a session which corresponds to a missing file.

Hello, in the current implementation, if a request contains a session token that has been stored in a file that has been deleted and we're trying to erase it using the Options.MaxAge = -1 workaround, the action still fails, because the missing file error gets propagated higher in the stack.

This small fix prevents this, and ensures that the session is regenerated.


This is a reopen of #237 which was closed by the stale bot.

@DavidLarsKetch
Copy link

Checks out to me, I recommend this is merged in after #253

@mariusor
Copy link
Contributor Author

Thank you for the support @DavidLarsKetch :) Are you affiliated with the project in an official capacity?

@DavidLarsKetch
Copy link

Not yet, just trying to lend a hand with review. I think @elithrar is still the only one with write permissions.

@stale
Copy link

stale bot commented Apr 27, 2022

This issue has been automatically marked as stale because it hasn't seen a recent update. It'll be automatically closed in a few days.

@stale stale bot added the stale label Apr 27, 2022
@stale stale bot removed the stale label Jul 7, 2023
@codecov
Copy link

codecov bot commented Aug 17, 2023

Codecov Report

Merging #252 (b0eac4a) into main (69327c5) will not change coverage.
The diff coverage is 0.00%.

@@           Coverage Diff           @@
##             main     #252   +/-   ##
=======================================
  Coverage   76.86%   76.86%           
=======================================
  Files           4        4           
  Lines         268      268           
=======================================
  Hits          206      206           
  Misses         54       54           
  Partials        8        8           
Files Changed Coverage Δ
store.go 78.01% <0.00%> (ø)

@coreydaley coreydaley merged commit dd83328 into gorilla:main Aug 17, 2023
8 of 9 checks passed
@mariusor
Copy link
Contributor Author

Whoa! Thank you @coreydaley 🥳

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants