Skip to content

Conversation

scottinet
Copy link
Contributor

@scottinet scottinet commented Feb 14, 2019

Description

The Kuzzle event tokenExpired should be triggered in two cases:

  • when a request response contains tokenExpired error
  • whenever a TokenExpired realtime event is received

The current SDK version only handles the first case, which causes many problems for clients using realtime, such as:

  • realtime callbacks are not cleared, meaning clients handling manually the event have more and more of the same callbacks registered (and invoked)
  • the tokenExpired event is not triggered upon receiving a TokenExpired realtime notification
  • the JWT is not cleared upon receiving a TokenExpired realtime notification

This PR adds a proper TokenExpired realtime notification handling, ensuring that all the above problems are solved.

@codecov-io
Copy link

codecov-io commented Feb 14, 2019

Codecov Report

Merging #374 into 6-dev will increase coverage by 0.03%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##            6-dev     #374      +/-   ##
==========================================
+ Coverage   96.61%   96.64%   +0.03%     
==========================================
  Files          28       28              
  Lines        1417     1430      +13     
==========================================
+ Hits         1369     1382      +13     
  Misses         48       48
Impacted Files Coverage Δ
src/controllers/realtime/room.js 100% <100%> (ø) ⬆️
src/controllers/realtime/index.js 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6397ca7...c3b34d0. Read the comment docs.

@alexandrebouthinon alexandrebouthinon merged commit d9c2832 into 6-dev Feb 14, 2019
@alexandrebouthinon alexandrebouthinon deleted the fix-tokenExpired-handling branch February 14, 2019 14:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants