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

Logout should always give a response. #369

Merged
merged 2 commits into from Dec 14, 2016

Conversation

Projects
None yet
3 participants
@marshallswain
Copy link
Member

marshallswain commented Dec 14, 2016

I'm not sure what the correct way to handle this is, but this fixes feathersjs/authentication-client#10.

Logout should always give a response.
I'm not sure what the correct way to handle this is, but this fixes feathersjs/authentication-client#10.
@daffl

This comment has been minimized.

Copy link
Member

daffl commented Dec 14, 2016

Great catch! Should this resolve with an empty object or throw an error?

@marshallswain

This comment has been minimized.

Copy link
Member Author

marshallswain commented Dec 14, 2016

The goal is to end up without an authenticated user, which is what you end up with even if you were never logged in. So I don't think it should throw an error. That's why I opted for the empty object.

Make sure it’s a function before trying to call it
I somehow encountered a scenario where callback didn’t exist, so this catches it.  For some reason I can’t seem to test logging out when not logged in.
@ekryski

This comment has been minimized.

Copy link
Member

ekryski commented Dec 14, 2016

Ya nice catch! Looks good. :shipit:

@marshallswain marshallswain merged commit bf59cf2 into master Dec 14, 2016

3 of 4 checks passed

codeclimate 13 new issues
Details
codeclimate/coverage 81.91% (-0.4%)
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@marshallswain marshallswain deleted the logout-should-resolve branch Dec 14, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.
You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session.