Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix: Small improvements #1605

Merged
merged 1 commit into from
Oct 7, 2019
Merged

Conversation

vonagam
Copy link
Member

@vonagam vonagam commented Oct 7, 2019

Small types and consistency style fixes. But also one real fix:

handleConnection method of AuthenticationBase and JWTStrategy was never called in response to disconnect event, so when a jwt expiration timer emitted disconnect connection.authentication was not removed and the timer was not cleared. Added listener for disconnect event to fix this. Also added expirationTimers.delete after a timer is cleared and lt.clearTimeout before a timer is set (to ensure that only one timer per connection exist at a time, otherwise multiple calls to reAuthenticate with force can create more than one timer).

@daffl daffl merged commit 19854d3 into feathersjs:master Oct 7, 2019
@daffl
Copy link
Member

daffl commented Oct 7, 2019

That makes sense, thanks! I thought this was being tested but I guess the test only covered that the authentication information had been removed.

@vonagam vonagam deleted the fix-minor-stuff-3 branch October 8, 2019 09:57
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants