Fetch a new request token as soon as the browser becomes online#18778
Merged
Fetch a new request token as soon as the browser becomes online#18778
Conversation
ChristophWurst
requested review from
gary-kim,
georgehrke,
juliusknorr,
kesselb and
rullzer
Member
|
O this is very neat. And also very useful for apps indeed! I guess 19 is good enough for this for now. |
Member
|
What about listening to |
Contributor
|
Conflicts 😨 |
Member
Author
We're not polling. We use the online event already to fetch a token when the network state changes. But we need to pool while online as request tokens time out after a certain time. |
ChristophWurst
force-pushed
the
fix/fetch-request-token-offline-online
branch
from
e4f4ae2 to
eb8aa6b
Compare
juliusknorr
reviewed
Jan 24, 2020
| console.info('session token successfully updated after resuming network') | ||
|
|
||
| // Let apps know we're online and requests will have the new token | ||
| emit('networkOnline', { |
Member
There was a problem hiding this comment.
Would be nice to document those somewhere
ChristophWurst
added
the
pending documentation
Member
|
@ChristophWurst can you rebase this? Then we can get this is :) |
ChristophWurst
force-pushed
the
fix/fetch-request-token-offline-online
branch
from
eb8aa6b to
bf2e282
Compare
gary-kim
approved these changes
Jan 27, 2020
Member
gary-kim
left a comment
gary-kim
left a comment
There was a problem hiding this comment.
Didn't know about the online event. Cool!
juliusknorr
added
4. to release
Member
|
needs another rebase 💃 |
ChristophWurst
force-pushed
the
fix/fetch-request-token-offline-online
branch
from
bf2e282 to
9d7f63b
Compare
Member
Author
|
Now be quick and merge this :P |
rullzer
force-pushed
the
fix/fetch-request-token-offline-online
branch
from
9d7f63b to
4538f08
Compare
Member
|
/compile amend / |
npmbuildbot-nextcloud
bot
force-pushed
the
fix/fetch-request-token-offline-online
branch
from
4538f08 to
ca3d6bc
Compare
ChristophWurst
force-pushed
the
fix/fetch-request-token-offline-online
branch
from
ca3d6bc to
6690e3b
Compare
Member
Author
|
Rebase cool, rebase fine |
This comment has been minimized.
This comment has been minimized.
Signed-off-by: Christoph Wurst <christoph@winzerhof-wurst.at>
ChristophWurst
force-pushed
the
fix/fetch-request-token-offline-online
branch
from
6690e3b to
d1886b5
Compare
ChristophWurst
removed
the
pending documentation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #16809
Debugging a few Sentry tickets I saw evidence of HTTP 412's -> CSRF check failing. Usually those are unexpected as we periodically fetch the current token, but if the client machine is paused or the browser goes offline, the CSRF token might not be fetched when the connection returns. Thus I added event handlers that can pause the background requests when there is no network connectivity and send off a request for the new token as soon as there is connectivity.
How to test
As this is something apps might have interest in, e.g. when to resume operation after offline as you have a dependency on the CSRF token, I added events emitted to the event bus where apps can listen to both the browser going offline as well as coming back online.
Do we want this in 18 already? Then we can backport.