search

ueberauth / guardian_db

Guardian DB integration for tracking tokens and ensuring logout cannot be replayed.
MIT License
368 stars 87 forks source link

feat(token_storage): return error reason #120

Closed cyberhck closed 3 years ago

cyberhck commented 3 years ago

Hello there, This is my first PR in elixir ecosystem so if I'm missing something, please let me know.

So I got stuck for almost a day because of :token_storage_failure and no clue how to proceed, what helped is when I modified guardian_db to log the reason out.

Currently it's eating the error reason and not telling user why.

Normally I'd be like: image

But because it's very important to not have this kind of error eating stuff, I want to return the reason instead.

Please have a look :slightly_smiling_face:

Hanspagh commented 3 years ago

@cyberhck ping

cyberhck commented 3 years ago

oh hello @Hanspagh I'm sorry, I got incredibly busy and hadn't had a chance to play around with elixir.

I'm not sure if I'll be able to get back to this any time soon as I'm working with golang primarily now, please feel free to close it or push new commits and merge.

If I do get some free time and this is still open, I'll try to add it, however I can't make any promises :wink:

yordis commented 3 years ago

@Hanspagh we may want to take this, but since introduces breaking changes from the caller, we should be careful and collect information about potential issues with Guardian as well abstraction as well

cyberhck commented 3 years ago

Hello there, I moved away from elixir and made up my mind that I'm not going to be doing elixir (I have my reasons), I'll probably also not going to be getting back to this PR feel free to do whatever you please with this PR :slightly_smiling_face:

yordis commented 3 years ago

This will break the caller pattern matching, therefore, it is a breaking change.

I'm not going to be doing elixir (I have my reasons)

Good luck in your adventure, also unnecessary parenthesis, we all have our reasons, best wishes.